Migrates to package:web and js_interop #2064
Conversation
josh-burton
requested review from
krystofwoldrich,
stefanosiano and
buenaflor
as code owners
|
hey, I see that this is for wasm compilation but I don't think we support symbolication for wasm yet |
|
This is blocking some customers. Happy to discuss how we can help! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2064 +/- ##
==========================================
- Coverage 95.25% 89.22% -6.03%
==========================================
Files 54 131 +77
Lines 1791 4353 +2562
==========================================
+ Hits 1706 3884 +2178
- Misses 85 469 +384 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thank you for contributing! I agree that we should migrate to this and also maybe release this in a major version bump.
Some small nitpicks & questions. Also, we need to satisfy CI in order to proceed.
- We are missing a changelog entry
- integration tests on macOS are failing apparently
- min-version tests are failing. Do we need to bump dart/flutter versions?
- analyze is failing.
dart format .needs to be run & changes committed
| @@ -101,3 +92,7 @@ class WebEnricherEventProcessor implements EnricherEventProcessor { | |||
| ); | |||
| } | |||
| } | |||
|
|
|||
| extension on web.Navigator { | |||
| external double? get deviceMemory; | |||
There was a problem hiding this comment.
Is this used anywhere? If not, we can remove it.
There was a problem hiding this comment.
It's currently used in the int? _getMemorySize() method. This API has been added to the web package so in a future update it can be removed.
| } | ||
|
|
||
| // See https://github.com/flutter/flutter/blob/414d9238720a3cde85475f49ce0ba313f95046f7/packages/flutter/lib/src/foundation/_capabilities_web.dart#L10 |
There was a problem hiding this comment.
We could also add the comment fron the link as decriptions.
// These values are set by the engine. They are used to determine if the
// application is using canvaskit or skwasm.
//
// See https://github.com/flutter/flutter/blob/414d9238720a3cde85475f49ce0ba313f95046f7/packages/flutter/lib/src/foundation/_capabilities_web.dart#L10
|
quick fyi: the ci job |
|
@denrase @buenaflor I've addressed the PR comments and hopefully fixed the build failures. I needed to bump the min version of Flutter has Dart 3.1.0 is required by package:web. |
|
@josh-burton if it's okay with you we'd like to fork your branch and take over from here, it's easier for us to fix the rest of the ci since we don't need to approve runnning them every time (cc @denrase) |
No problem, go for it. |
|
Any help we can get here? |
📜 Description
Migrates to package:web and js_interop
💡 Motivation and Context
Supports compilation via WASM
💚 How did you test it?
Tested on a flutter web app project, ran unit tests
📝 Checklist
sendDefaultPiiis enabled🔮 Next steps
Version numbers should be bumped. I think technically this is not a breaking change, but perhaps it should be treated as one.