Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBarrel exports in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ndk/lib/ndk.dart`:
- Line 69: The root export in ndk.dart exposes
data_layer/repositories/cache_manager/sembast_cache_manager.dart which imports
dart:io and package:sembast/sembast_io.dart and breaks web consumers; fix by
removing the direct export and either (a) replace it with a conditional export
using the if (dart.library.io) syntax in ndk.dart to export
sembast_cache_manager for IO platforms and mem_cache_manager as the default, or
(b) move the Sembast implementation into a platform-specific entrypoint (e.g., a
new package:ndk/cache_managers/io.dart) and export only the web-safe
mem_cache_manager from the root barrel, ensuring references to
sembast_cache_manager.dart and mem_cache_manager are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00079ae6-7605-4ee3-82fa-532d102f5a8b
⛔ Files ignored due to path filters (1)
packages/sample-app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
packages/ndk/example/sembast/CLAUDE.mdpackages/ndk/example/sembast/README.mdpackages/ndk/example/sembast/sembast_cache_manager_example.dartpackages/ndk/lib/data_layer/repositories/cache_manager/ndk_extensions.dartpackages/ndk/lib/data_layer/repositories/cache_manager/sembast_cache_manager.dartpackages/ndk/lib/ndk.dartpackages/ndk/test/data_layer/cache_manager/sembast_cache_manager_test.dartpackages/sample-app/lib/main.dartpackages/sample-app/pubspec.yamlpackages/sembast_cache_manager/.gitignorepackages/sembast_cache_manager/CHANGELOG.mdpackages/sembast_cache_manager/LICENSEpackages/sembast_cache_manager/README.mdpackages/sembast_cache_manager/analysis_options.yamlpackages/sembast_cache_manager/lib/sembast_cache_manager.dartpackages/sembast_cache_manager/pubspec.yaml
💤 Files with no reviewable changes (10)
- packages/sembast_cache_manager/LICENSE
- packages/sembast_cache_manager/.gitignore
- packages/sembast_cache_manager/CHANGELOG.md
- packages/sembast_cache_manager/analysis_options.yaml
- packages/sembast_cache_manager/lib/sembast_cache_manager.dart
- packages/sample-app/lib/main.dart
- packages/sembast_cache_manager/pubspec.yaml
- packages/sample-app/pubspec.yaml
- packages/sembast_cache_manager/README.md
- packages/ndk/example/sembast/sembast_cache_manager_example.dart
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #524 +/- ##
==========================================
+ Coverage 73.36% 74.24% +0.88%
==========================================
Files 195 197 +2
Lines 8969 9561 +592
==========================================
+ Hits 6580 7099 +519
- Misses 2389 2462 +73 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ndk/lib/ndk.dart (1)
47-47: Remove empty export statement.
export '';is dead code that serves no purpose and should be removed.🧹 Proposed fix
-export '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ndk/lib/ndk.dart` at line 47, Remove the dead empty export statement `export '';` from the ndk.dart module; locate the line containing that exact export in the ndk.dart file and delete it so the file does not contain a no-op export.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ndk/lib/ndk.dart`:
- Around line 69-70: The conditional export in ndk.dart currently points to the
same file for both branches, causing web builds to pull in dart:io; update the
export to provide a web-safe stub: change the export to use
'data_layer/repositories/cache_manager/sembast_cache_manager.dart' when
dart.library.io is true and to a new
'data_layer/repositories/cache_manager/sembast_cache_manager_stub.dart' when
false, and create that stub file implementing the same public API
(classes/functions) from sembast_cache_manager.dart but with bodies that throw
UnsupportedError (or no-op) on web so compilation succeeds; ensure public
symbols (e.g., the cache manager class and any factory functions) match names in
the original file.
---
Nitpick comments:
In `@packages/ndk/lib/ndk.dart`:
- Line 47: Remove the dead empty export statement `export '';` from the ndk.dart
module; locate the line containing that exact export in the ndk.dart file and
delete it so the file does not contain a no-op export.
| export 'data_layer/repositories/cache_manager/sembast_cache_manager.dart' | ||
| if (dart.library.io) 'data_layer/repositories/cache_manager/sembast_cache_manager.dart'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for dart:io and sembast_io imports in the sembast cache manager
fd -t f "sembast_cache_manager.dart" packages/ndk --exec head -50 {}
echo "---"
echo "Searching for dart:io imports:"
rg -n "dart:io|sembast_io" packages/ndk/lib/data_layer/repositories/cache_manager/Repository: relaystr/ndk
Length of output: 2676
Conditional export with identical paths will break web builds.
The file sembast_cache_manager.dart imports dart:io and package:sembast/sembast_io.dart, which are unavailable on web. The current conditional export uses the same path for both conditions, so when dart.library.io is false (web builds), it still attempts to export sembast_cache_manager.dart, causing compilation failure.
Fix: Create a stub implementation for web or remove the unconditional default:
-export 'data_layer/repositories/cache_manager/sembast_cache_manager.dart'
- if (dart.library.io) 'data_layer/repositories/cache_manager/sembast_cache_manager.dart';
+export 'data_layer/repositories/cache_manager/sembast_cache_manager_stub.dart'
+ if (dart.library.io) 'data_layer/repositories/cache_manager/sembast_cache_manager.dart';Then create sembast_cache_manager_stub.dart with placeholder implementations that throw UnsupportedError if called on web.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ndk/lib/ndk.dart` around lines 69 - 70, The conditional export in
ndk.dart currently points to the same file for both branches, causing web builds
to pull in dart:io; update the export to provide a web-safe stub: change the
export to use 'data_layer/repositories/cache_manager/sembast_cache_manager.dart'
when dart.library.io is true and to a new
'data_layer/repositories/cache_manager/sembast_cache_manager_stub.dart' when
false, and create that stub file implementing the same public API
(classes/functions) from sembast_cache_manager.dart but with bodies that throw
UnsupportedError (or no-op) on web so compilation succeeds; ensure public
symbols (e.g., the cache manager class and any factory functions) match names in
the original file.
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| package: [sembast_cache_manager, objectbox, drift] |
There was a problem hiding this comment.
how is sembast tested now?
Directly in the core package?
There was a problem hiding this comment.
Yes, in same test package as mem_cache_manager_test
3 participants
Summary by CodeRabbit