fix(llc): Fixed build in dart2wasm environment #2379
Conversation
WalkthroughReplaced conditional imports/exports from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant Selector as "Conditional selector\n(if dart.library.js_interop)"
participant WebImpl as "Web implementation"
participant IOImpl as "IO implementation"
App->>Selector: Request platform-specific module
alt dart.library.js_interop present
Selector->>WebImpl: Load/export web module
WebImpl-->>App: Web implementation available
else
Selector->>IOImpl: Load/export IO module
IOImpl-->>App: IO implementation available
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🧹 Nitpick comments (2)
packages/stream_chat/lib/src/core/platform_detector/platform_detector.dart (1)
2-2: Tiny doc hint for future readers.Consider a short comment explaining why js_interop (not html) is the web selector (Wasm compatibility).
- if (dart.library.js_interop) 'platform_detector_web.dart' + // Use js_interop as the web selector to support both JS and Wasm runtimes. + if (dart.library.js_interop) 'platform_detector_web.dart'packages/stream_chat_flutter/lib/src/attachment/handler/stream_attachment_handler.dart (1)
2-3: *Gate change LGTM; consider renaming the selected file to _web.dart for clarity.Since selection now keys off js_interop (Web JS/Wasm), stream_attachment_handler_html.dart is a misnomer. Optional rename to stream_attachment_handler_web.dart (if feasible).
-export 'stream_attachment_handler_base.dart' - if (dart.library.js_interop) 'stream_attachment_handler_html.dart' +export 'stream_attachment_handler_base.dart' + if (dart.library.js_interop) 'stream_attachment_handler_web.dart' if (dart.library.io) 'stream_attachment_handler_io.dart' show StreamAttachmentHandler;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/stream_chat/lib/src/core/platform_detector/platform_detector.dart(1 hunks)packages/stream_chat_flutter/lib/src/attachment/handler/stream_attachment_handler.dart(1 hunks)packages/stream_chat_persistence/lib/src/db/shared/shared_db.dart(1 hunks)
🔇 Additional comments (3)
packages/stream_chat/lib/src/core/platform_detector/platform_detector.dart (2)
2-3: Confirm platform_detector_web.dart avoids dart:htmlVerification script produced no output; cannot confirm. Verify packages/stream_chat/lib/src/core/platform_detector/platform_detector_web.dart does not import 'dart:html' or 'package:html' and instead imports 'dart:js_interop' or 'package:web'.
2-2: SDK constraints already cover dart.library.js_interopAll package pubspecs use sdk: ^3.6.2 (Dart ≥3.6) and Flutter constraints where present are >=3.27.4; dart.library.js_interop is supported since Dart 3.1 / Flutter 3.22 — no changes required.
packages/stream_chat_flutter/lib/src/attachment/handler/stream_attachment_handler.dart (1)
2-2: stream_attachment_handler_html.dart — no dart:html imports (resolved)packages/stream_chat_flutter/lib/src/attachment/handler/stream_attachment_handler_html.dart does not import 'dart:html' or reference DOM-only types (it only uses FilePicker/XFile APIs); repo-wide search found no dart:html/package:web/package:js usages and the conditional export (if (dart.library.js_interop) ...) is correct.
| export 'unsupported_db.dart' | ||
| if (dart.library.io) 'native_db.dart' // implementation using dart:io | ||
| if (dart.library.html) 'web_db.dart'; | ||
| if (dart.library.js_interop) 'web_db.dart'; |
There was a problem hiding this comment.
🧩 Analysis chain
Good switch; confirm web_db.dart is Wasm‑safe (no dart:html or non‑Wasm APIs).
If web_db.dart still uses dart:html or html-only storage APIs, this gate will break Wasm. Ensure it uses the Wasm-friendly drift/web/wasm path.
🏁 Script executed:
#!/bin/bash
# Ensure persistence web impl avoids dart:html
rg -nC2 -e "^import\s+['\"]dart:html['\"]" packages/stream_chat_persistence/lib/src/db/shared/web_db.dart || true
# Grep for drift wasm/web connectors (adjust patterns to your impl)
rg -nC2 -e "drift.*wasm" -e "import\s+['\"][^'\"]*drift/web" packages/stream_chat_persistence/lib/src/db/shared/web_db.dart || trueLength of output: 273
Do not approve — web_db.dart is not Wasm-safe; implement a Wasm connector
packages/stream_chat_persistence/lib/src/db/shared/web_db.dart still imports 'package:drift/web.dart' and has "// TODO: Replace with WASM implementation"; this uses web-only APIs and will break the Wasm gate. Replace with a Wasm-friendly connector (e.g., drift/web/wasm) or a drift wasm implementation that avoids dart:html, then re-verify.
packages/stream_chat/lib/src/core/platform_detector/platform_detector.dart
Show resolved
Hide resolved
packages/stream_chat_flutter/lib/src/attachment/handler/stream_attachment_handler.dart
Show resolved
Hide resolved
|
@xsahil03x Running it via 
But running it with 
( If you want to test |
|
Thanks, I will soon test it locally. Maybe we want to wait for the drift PR to be released first before merging this one |
|
Thanks. Just to note it's not blocked to drift's PR (which is merged now but I understand you said a release is needed not just the merge) as it's possible to test And drift's PR isn't about js_interop but about use of (and I don't know whether this is the best solution or not and am not pushing for it on any cost just that found this solution from https://dart.dev/interop/js-interop/past-js-interop (but inconsistently?) and found to be useful for the case here) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2379 +/- ##
=======================================
Coverage 63.76% 63.76%
=======================================
Files 412 412
Lines 25821 25821
=======================================
Hits 16466 16466
Misses 9355 9355 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ebraminio
changed the title
ebraminio
changed the title
ebraminio
changed the title
According to https://dart.dev/interop/js-interop/past-js-interop we should now "prefer using dart:js_interop going forwards and migrate usages of old interop libraries when possible" and this change does that in order to make the library compatible with `--wasm` builds.
According to https://dart.dev/interop/js-interop/past-js-interop we should now "prefer using dart:js_interop going forwards and migrate usages of old interop libraries when possible" and this change does that in order to make the library compatible with
--wasmbuilds.Submit a pull request
Linear: FLU-
Github Issue: #
CLA
--wasmon this project needs Avoid incorrect cast in sql_js in wasm builds simolus3/drift#3657 (already merged but not released) and Migrate to package:web Alberto-Monteiro/video_thumbnail#9 also but having all of them it's possible to make it in the CI also AFAICS.Description of the pull request
When I'm building example of this project in
flutter run -d chrome --wasmmode the web mode isn't recognized correctly and raises errors like this on the consoleWhich this change fixes
Summary by CodeRabbit
Bug Fixes
Refactor
Chores