Import dart:_js_interop_wasm in addition to dart:_wasm to convert between JSAny and WasmExternRef?#186974
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request adds the dart:_js_interop_wasm import to raw_image.dart, raw_surface.dart, and surface.dart within the skwasm_impl directory. These additions include analyzer ignores and a TODO comment referencing a Dart issue. I have no feedback to provide.
|
autosubmit label was removed for flutter/flutter/186974, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
|
Could I get a second reviewer for this please? 🙏 |
|
autosubmit label was removed for flutter/flutter/186974, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
|
Thanks for updating this @harryterkelsen 🙏 I think this only needs an autosubmit label now? |
flutter/flutter@3a0420c...b10d0f1 2026-06-17 [email protected] Skip platform-specific plugin registration if no platforms enabled (flutter/flutter#186304) 2026-06-17 [email protected] Roll Packages from 8286d39 to 6ce00a8 (1 revision) (flutter/flutter#188109) 2026-06-17 [email protected] Roll Skia from 79f93fd5f36e to 5d19002eb73e (1 revision) (flutter/flutter#188108) 2026-06-17 [email protected] Import `dart:_js_interop_wasm` in addition to `dart:_wasm` to convert between `JSAny` and `WasmExternRef?` (flutter/flutter#186974) 2026-06-17 [email protected] Roll Dart SDK from f811ecae9ca0 to e39bde5b1bfc (2 revisions) (flutter/flutter#188107) 2026-06-17 [email protected] Roll Skia from 026f6a6be2b9 to 79f93fd5f36e (1 revision) (flutter/flutter#188105) 2026-06-17 [email protected] Roll Dart SDK from 462bf0a1d489 to f811ecae9ca0 (1 revision) (flutter/flutter#188099) 2026-06-17 [email protected] Roll Fuchsia Linux SDK from VeLhhlDcod09NR4Hb... to or21OEdGtairm6nl9... (flutter/flutter#188098) 2026-06-17 [email protected] Roll Skia from 2ffd155313f5 to 026f6a6be2b9 (10 revisions) (flutter/flutter#188097) 2026-06-17 [email protected] Roll Dart SDK from 824b4b48b6d4 to 462bf0a1d489 (1 revision) (flutter/flutter#188093) 2026-06-17 [email protected] Manual Dart roll from f6c31f4c3a63 to 824b4b48b6d4 (flutter/flutter#188023) 2026-06-17 [email protected] Add a platform view test to android_hardware_smoke_test (flutter/flutter#188069) 2026-06-17 [email protected] [flutter_tools] Format empty app template with latest dart format (flutter/flutter#187443) 2026-06-16 49699333+dependabot[bot]@users.noreply.github.com Bump the all-github-actions group across 1 directory with 3 updates (flutter/flutter#188086) 2026-06-16 [email protected] Roll Skia from d7196b0b4939 to 2ffd155313f5 (9 revisions) (flutter/flutter#188081) 2026-06-16 [email protected] Prevent downgrading `project.pbxproj` when greater version number (flutter/flutter#186250) 2026-06-16 [email protected] Only allow dependabot to autoupdate GitHub-owned actions (flutter/flutter#187936) 2026-06-16 [email protected] Fall back to source AndroidManifest.xml if AAPT fails or returns garbage (flutter/flutter#187197) 2026-06-16 [email protected] Roll pub packages (flutter/flutter#187769) 2026-06-16 [email protected] [Impeller] Move queue submission into a callback that is invoked by FenceWaiterVK::AddFence only if it can accept the fence (flutter/flutter#187761) 2026-06-16 [email protected] Reland [a11y] Map some framework semantics roles to android classes. (flutter/flutter#188037) 2026-06-16 [email protected] refactor(web): Unify Image on Skwasm and CanvasKit (flutter/flutter#187873) 2026-06-16 [email protected] Adds arm64 variant of impeller devicelab tests for windows. (flutter/flutter#188053) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
… between `JSAny` and `WasmExternRef?` (flutter#186974) Currently, `dart:_wasm` exports `externRefForJSAny` and `WasmExternRefToJSAny.toJS` functions and extension getters to convert between `JSAny` and `WasmExternRef?`. Flutter relies on these exports in its `skwasm` implementation. We want to refactor `dart:_wasm` to not export these members from anymore (dart-lang/sdk#63166). The reason is that there'll be a standalone WebAssembly target for Dart, which supports `dart:_wasm` but not `dart:js_interop`. To be able to remove these methods, we've added a `dart:_js_interop_wasm` library defining these helpers (dart-lang/sdk@3196571). Because we're not allowed to break `dart analyze` in Flutter, we need to do this migration in stages. `dart:_wasm` currently re-exports `dart:_js_interop_wasm`, only for this reason. To be able to remove that export, Flutter needs to import `dart:_js_interop_wasm` directly. Once this PR is merged, we can drop the export in the Dart SDk and then remove the `unnecessary_import` lint (`import_internal_library` is an oversight, the analyzer should special-case that library just like `dart:_wasm` itself). ### Tests This doesn't actually change anything, `dart:_wasm` and `dart:_js_interop_wasm` export the same members (everything else would be a compilation error). I've manually ran `dart analyze` with an SDK built from [this change I want to land afterwards](https://dart-review.googlesource.com/c/sdk/+/505960) to ensure Flutter builds correctly once we really remove the export. So I think I'll need a test-exempt status here. Closes flutter#186973. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Kevin Moore <[email protected]> Co-authored-by: Harry Terkelsen <[email protected]>
… between `JSAny` and `WasmExternRef?` (flutter#186974) Currently, `dart:_wasm` exports `externRefForJSAny` and `WasmExternRefToJSAny.toJS` functions and extension getters to convert between `JSAny` and `WasmExternRef?`. Flutter relies on these exports in its `skwasm` implementation. We want to refactor `dart:_wasm` to not export these members from anymore (dart-lang/sdk#63166). The reason is that there'll be a standalone WebAssembly target for Dart, which supports `dart:_wasm` but not `dart:js_interop`. To be able to remove these methods, we've added a `dart:_js_interop_wasm` library defining these helpers (dart-lang/sdk@3196571). Because we're not allowed to break `dart analyze` in Flutter, we need to do this migration in stages. `dart:_wasm` currently re-exports `dart:_js_interop_wasm`, only for this reason. To be able to remove that export, Flutter needs to import `dart:_js_interop_wasm` directly. Once this PR is merged, we can drop the export in the Dart SDk and then remove the `unnecessary_import` lint (`import_internal_library` is an oversight, the analyzer should special-case that library just like `dart:_wasm` itself). ### Tests This doesn't actually change anything, `dart:_wasm` and `dart:_js_interop_wasm` export the same members (everything else would be a compilation error). I've manually ran `dart analyze` with an SDK built from [this change I want to land afterwards](https://dart-review.googlesource.com/c/sdk/+/505960) to ensure Flutter builds correctly once we really remove the export. So I think I'll need a test-exempt status here. Closes flutter#186973. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Kevin Moore <[email protected]> Co-authored-by: Harry Terkelsen <[email protected]>
This is a follow-up for [0] which should unblock landing [1] (which currently fails with g3 roll error). [0] flutter#186974 [1] https://dart-review.googlesource.com/c/sdk/+/505960
…88620) This is a follow-up for [0] which should unblock landing [1] (which currently fails with g3 roll error). [0] flutter#186974 [1] https://dart-review.googlesource.com/c/sdk/+/505960
`dart:_wasm` exports `dart:_js_interop_wasm` because Flutter relies on it. With flutter/flutter#186974 merged, we can stop exporting that in the SDK. Flutter and others needing to convert between JS interop types and `WasmExternRef` must import `dart:_js_interop_wasm` directly. This allows removing `dart:js_interop` from the standalone target entirely, which makes sense as that general-purpose WebAssembly target should not assume JavaScript to be available. Cq-Include-Trybots: luci.dart.try:flutter-analyze-try Change-Id: I69c6ea5cd57a2e1928b9faf977946762acfbcbe4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505960 Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Kevin Moore <[email protected]>
Currently,
dart:_wasmexportsexternRefForJSAnyandWasmExternRefToJSAny.toJSfunctions and extension getters to convert betweenJSAnyandWasmExternRef?. Flutter relies on these exports in itsskwasmimplementation.We want to refactor
dart:_wasmto not export these members from anymore (dart-lang/sdk#63166). The reason is that there'll be a standalone WebAssembly target for Dart, which supportsdart:_wasmbut notdart:js_interop. To be able to remove these methods, we've added adart:_js_interop_wasmlibrary defining these helpers (dart-lang/sdk@3196571). Because we're not allowed to breakdart analyzein Flutter, we need to do this migration in stages.dart:_wasmcurrently re-exportsdart:_js_interop_wasm, only for this reason.To be able to remove that export, Flutter needs to import
dart:_js_interop_wasmdirectly. Once this PR is merged, we can drop the export in the Dart SDk and then remove theunnecessary_importlint (import_internal_libraryis an oversight, the analyzer should special-case that library just likedart:_wasmitself).Tests
This doesn't actually change anything,
dart:_wasmanddart:_js_interop_wasmexport the same members (everything else would be a compilation error). I've manually randart analyzewith an SDK built from this change I want to land afterwards to ensure Flutter builds correctly once we really remove the export.So I think I'll need a test-exempt status here.
Closes #186973.
Pre-launch Checklist
///).