-
-
Notifications
You must be signed in to change notification settings - Fork 214
fix: Incompatible parseIsWeb detection prevents WASM support
#1096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
Replace `identical(0, 0.0)` with `bool.fromEnvironment('dart.library.js_util')` for proper web platform detection that works with WASM.
Co-authored-by: mtrezza <[email protected]>
|
I will reformat the title to use the proper commit message syntax. |
parseIsWeb detection prevents WASM compatibility
|
🚀 Thanks for opening this pull request! |
parseIsWeb detection prevents WASM compatibilityparseIsWeb detection prevents WASM support
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1096 +/- ##
=======================================
Coverage 43.96% 43.96%
=======================================
Files 61 61
Lines 3596 3596
=======================================
Hits 1581 1581
Misses 2015 2015 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot Do you recommend to extend the CI for testing in WASM? |
Yes, I recommend adding WASM testing to CI, but it can be done as a follow-up. For the Dart package, you could add a job that compiles to WASM and runs tests using However, this change is relatively low-risk since I'd suggest opening a separate issue to track adding WASM-specific CI jobs when the team is ready to expand platform coverage. |
|
@copilot Is there more needed to expand platform coverage to WASM, or would it be enough to simply extend the matrix in ci.yml with a WASM compilation? |
For the Flutter package, it would be straightforward - you could add a matrix entry with a For the Dart package, it's more complex. The current
Given this SDK is primarily used via Flutter (not standalone Dart web apps), extending the Flutter CI matrix with |
|
@copilot then extend the Flutter CI matrix with --wasm |
# [dart-v9.4.5](dart-9.4.4...dart-9.4.5) (2025-12-04) ### Bug Fixes * Incompatible `parseIsWeb` detection prevents WASM support ([#1096](#1096)) ([5b157b8](5b157b8))
|
🎉 This change has been released in version dart-v9.4.5 |
|
@copilot Open a new PR to add wasm to the test matrix. |
Pull Request
Issue
Closes: #1058
Approach
The
identical(0, 0.0)trick for web detection fails on WASM because WASM properly distinguishes integers from doubles (unlike JavaScript where they're unified as Number).Replace with the standard Dart approach:
This works correctly for JavaScript, WASM, and native targets.
Tasks
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.