Sitelet https://github.com/flutter/flutter/pull/183835
Skip to content

Use dart2wasm app.support.js expression with js-string builtin requirement - #183835

Merged
mkustermann merged 3 commits into
flutter:masterfrom
mkustermann:dart2wasm-support-expr
Aug 5, 2026
Merged

mkustermann merged 3 commits into
flutter:masterfrom
mkustermann:dart2wasm-support-expr

Conversation

@mkustermann

@mkustermann mkustermann commented Mar 18, 2026 •

Copy link
Copy Markdown
Member

This changes flutter web's detection of wasm compatibility to be the expression emitted by dart2wasm.

The expression was generated by passing --require-js-string-builtin to the dart2wasm compiler. Newer versions of browsers all support the js-string builtins nowadays. Apps running on browsers without js-string builtin support will not run in a performant way, so we'd not want to run the wasm version on them anyway.

This is a preparation for dart2wasm to require the js-string builtin by default (i.e. we'll remove the --require-js-string-builtin flag and also remove the polyfill for js-string builtins). See [0]

[0] https://dart-review.googlesource.com/c/sdk/+/488840

For reference of Safari testing on Dart:

@flutter-dashboard

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Mar 18, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the WebAssembly compatibility check by replacing the supportsWasmGC function with a more comprehensive supportsDart2Wasm function. This new function checks for both WasmGC and js-string builtin support, which is a requirement for modern dart2wasm output. The changes are consistently applied across the JavaScript logic, its corresponding TypeScript definitions, and the loader.

@flutter flutter deleted a comment from gemini-code-assist Bot Mar 18, 2026
Comment thread engine/src/flutter/lib/web_ui/flutter_js/src/browser_environment.js Outdated

@mdebbar mdebbar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some questions and suggestions for leaving breadcrumbs.

I'll defer to @eyebrowsoffire to give the final approval here.

Comment thread engine/src/flutter/lib/web_ui/flutter_js/src/browser_environment.js Outdated
Comment thread engine/src/flutter/lib/web_ui/flutter_js/src/browser_environment.js Outdated
Comment thread engine/src/flutter/lib/web_ui/flutter_js/src/browser_environment.js Outdated
@mdebbar mdebbar added the CICD Run CI/CD label Mar 18, 2026
@mdebbar

mdebbar commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

I believe the .ci.yaml validation error can be resolved by rebasing.

@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 19, 2026
@mkustermann
mkustermann force-pushed the dart2wasm-support-expr branch from 2ecab18 to ff74902 Compare March 19, 2026 09:22
@mkustermann

Copy link
Copy Markdown
Member Author

I believe the .ci.yaml validation error can be resolved by rebasing.

👍 Done

@mkustermann

Copy link
Copy Markdown
Member Author

@eyebrowsoffire Could you have a look if you have some time?

@mkustermann
mkustermann force-pushed the dart2wasm-support-expr branch from ff74902 to bbfba85 Compare March 20, 2026 11:56
eyebrowsoffire
eyebrowsoffire previously approved these changes Apr 28, 2026

@eyebrowsoffire eyebrowsoffire left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mkustermann
mkustermann force-pushed the dart2wasm-support-expr branch from bbfba85 to 849adea Compare April 28, 2026 18:56
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Apr 28, 2026
mdebbar
mdebbar previously approved these changes May 18, 2026

@mdebbar mdebbar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkustermann is this still something we want to land?

@mkustermann

Copy link
Copy Markdown
Member Author

@mkustermann is this still something we want to land?

Absolutely. Though there's some failures from flutter CI bots I'd need to dig into (in case someone from flutter knows already what's wrong, please let me know!).

Before doing so, I decided to first add proper Safari tests on Dart CI first (which is still in-progress, as things always turn out to be more complicated then they should be). So far we have been only testing on the JS commandline version of Safari and only the newest version of it (i.e. bleeding edge JavaScriptCore JS shell).

@kevmoo

kevmoo commented May 18, 2026

Copy link
Copy Markdown
Contributor

Would you add a link to the top (original) PR comment linking to the pending Dart CL? Just so we keep our relationships straight. 🙏

@mkustermann

Copy link
Copy Markdown
Member Author

Would you add a link to the top (original) PR comment linking to the pending Dart CL? Just so we keep our relationships straight. 🙏

Done

@kevmoo

kevmoo commented May 27, 2026

Copy link
Copy Markdown
Contributor

I believe this is blocked by https://github.com/flutter/flutter/pull/182861 – we need latest Chrome in our framework tests!

@flutter-dashboard

Copy link
Copy Markdown

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@gaaclarke

Copy link
Copy Markdown
Member

hey @mkustermann are you still interested in this PR? Just a heads up that it failed on CI, it sounds like you guys were waiting for some sort of chrome update that maybe is available now.

This changes flutter web's detection of wasm compatibility to be the
expression emitted by dart2wasm.

The expression was generated by passing `--require-js-string-builtin` to
the dart2wasm compiler. Newer versions of browsers all support the
`js-string` builtins nowadays. Apps running on browsers without
`js-string` builtin support will not run in a performant way, so we'd
not want to run the wasm version on them anyway.

This is a preparation for dart2wasm to require the `js-string` builtin
by default (i.e. we'll remove the `--require-js-string-builtin` flag and
also remove the polyfill for `js-string` builtins).
@mkustermann
mkustermann dismissed stale reviews from mdebbar and eyebrowsoffire via 512fa04 July 28, 2026 08:07
@mkustermann
mkustermann force-pushed the dart2wasm-support-expr branch from 849adea to 512fa04 Compare July 28, 2026 08:07
@github-actions github-actions Bot added the team-web Owned by Web platform team label Jul 28, 2026
@mkustermann

Copy link
Copy Markdown
Member Author

are you still interested in this PR? Just a heads up that it failed on CI, it sounds like you guys were waiting for some sort of chrome update that maybe is available now.

Thanks for the ping, @gaaclarke ! Yes, I'd like the PR to land, 🤞 that the Chrome update will make CI pass now

Rebased now.

@mkustermann

Copy link
Copy Markdown
Member Author

@eyebrowsoffire It seems it needs another LGTM

@mkustermann
mkustermann enabled auto-merge July 29, 2026 07:50
@mkustermann

Copy link
Copy Markdown
Member Author

@mdebbar Could you give another lgtm?

@mkustermann
mkustermann added this pull request to the merge queue Aug 5, 2026
Merged via the queue into flutter:master with commit 4c5bf76 Aug 5, 2026
16 of 17 checks passed
@mkustermann
mkustermann deleted the dart2wasm-support-expr branch August 5, 2026 22:23
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 6, 2026
For Dart String operations to be performant we require `js-string`
builtins - the polyfill is not going to be ver fast.

The newest versions of all major browsers support this now.

This removes the polyfill for `js-string` builtins and makes the
`<app>.support.js` code test for presence of `js-string` builtins.

The PR in [0] changes flutter engine wasm support detection code
to check for `js-string` builtins.

[0] flutter/flutter#183835

Change-Id: Id20996674e2f6ce421f679747f7c2694260aedbe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488840
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-web Web applications specifically team-web Owned by Web platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants