Skip to content

Add dart2wasm --dry-run option #60050

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

Open
yjbanov opened this issue Feb 4, 2025 · 8 comments
Open

Add dart2wasm --dry-run option #60050

yjbanov opened this issue Feb 4, 2025 · 8 comments
Assignees
Labels
area-dart2wasm Issues for the dart2wasm compiler.

Comments

@yjbanov
Copy link

yjbanov commented Feb 4, 2025

Because we see clear performance benefits across nearly every kind of Flutter Web app, let's encourage the ecosystem towards WebAssembly. One way to do this is by analyzing the Dart code that's compiled into the app, and reporting incompatibilities with WebAssembly.

Proposal

Add a new --dry-run option to dart2wasm. When specified, the compiler runs just enough of its pipeline to tell if it's able to compile the code cleanly.

If the code is compiled cleanly, the compiler exits with no output and exit code 0.

If the code fails to compile, the compiler exits with exit code 1 and output a report to stdout explaining where the incompatibilities are.

To maximize the benefit of the report, it should be:

  • Easily human-readable. Let's keep the level of detail in the report itself low.
    • If more detail is useful, let's assign a unique code to each kind of incompatibility and have the CLI tool link to website docs.
  • Incompatibilities in the app code should be grouped together and easy to find in the code.
  • Incompatibilities in packages should be grouped by package, so it's easy to copy & paste the list of incompatibilities from the report into a GitHub issue.

Sample report:

Found incompatibilities with WebAssembly.

Incompatibilities in the application:

lib/main.dart 2:1 - 'dart:html' unsupported (error 1234)
lib/src/foo.dart 3:1 - 'dart:js' unsupported (error 2345)

Incompatibilities in 'package:foo':

package:foo/src/bar.dart 23:45 - 'package:js' unsupported (error 3456)
package:foo/src/baz.dart 67:89 - 'package:js_util' unsupported (error 4567)

Incompatibilities in 'package:qux':

package:qux/src/bar.dart 23:45 - 'package:js' unsupported (error 3456)
package:qux/src/baz.dart 67:89 - 'package:js_util' unsupported (error 4567)

The Flutter CLI tool can enhance this report further by turning error codes into links.

@yjbanov yjbanov added the area-dart2wasm Issues for the dart2wasm compiler. label Feb 4, 2025
@mkustermann
Copy link
Member

mkustermann commented Feb 11, 2025

Setting aside how any incompatibilities would be reported, we'll first have to determine what criteria to use when we determine whether to compile an app with wasm (by default, without users opting in).

There's obvious and hard requirements: The app cannot use forbidden libraries like dart:html or packages package:js.

Though there's also more semantic things. From a recent look at how customers migrate code to static interop, it becomes apparent that some users migrate code the wrong way. It may even compile with wasm, but it clearly has issues. From recent example:

JSAny? jsValue;
if (jsValue is String) {
  ...
} else if (jsValue is JSArray) {
  ...
}

This code is semantically clearly wrong, e.g. A JSAny can never be a String - but it's valid Dart code, it compiles and it happens to run somewhat incorrectly in dart2js.

Now it seems the analyzer has implemented an opt-in hint - invalid_runtime_check_with_js_interop_types for this particular situation. Though the CFE may not be capable of warning / erroring about this (@johnniwinther would know?).

If we wanted to not auto-opt code into wasm if it has such semantic errors (which wouldn't be compile-time errors), then: Would we want to run analyzer + CFE over the code base? That seems not very ideal. Maybe we could add such warning to CFE? (I do find it actually weird that this is an opt-in -- why aren't we always warning about such problematic JS interop code?)

/cc @dart-lang/dart2js-team for discussion

@johnniwinther
Copy link
Member

Currently some jsinterop checking is done through the CFE pipeline, but these are mostly at the declarational level. To perform this kind of check we would need to add a hook into our assignment check during inference, which could be done.

@mkustermann
Copy link
Member

@srujzs As you've mainly worked on the new JS interop, would this be something you could own?

@srujzs
Copy link
Contributor

srujzs commented Feb 11, 2025

(I do find it actually weird that this is an opt-in -- why aren't we always warning about such problematic JS interop code?)

It's in the recommended list but not in the core list, because I was worried about false positives: dart-lang/core#824.

As you've mainly worked on the new JS interop, would this be something you could own?

Sure! I can take this on (although I probably won't get around to it/other interop stuff again until Q2), but we should enumerate what else we want from this list besides incompatible libraries and bad type-checks. @sigmundch has been looking at the discrepancies and what it might take to move more of the world to dart2wasm - maybe there are other things we want here.

Re: the lint, do we see enough of a benefit from re-implementing it in the CFE? Can we not just pass some logger to the specific lint and the JS interop checks and provide the resulting logs in the report?

@sigmundch
Copy link
Member

Re gaps/discrepancies - thanks for calling that out @srujzs, I just shared my doc with @yjbanov, @mkustermann and @eyebrowsoffire as well to provide more context.

Re lints - I agree. I was just chatting yesterday with @natebiggs about it too, I think most of our lints could be added to the existing _js_interop_checks modular transformer we run in all of our backends.

@mkustermann
Copy link
Member

Re: the lint, do we see enough of a benefit from re-implementing it in the CFE? Can we not just pass some logger to the specific lint and the JS interop checks and provide the resulting logs in the report?

Yeah, maybe that'd be easiest. (In ideal world the Analyzer and CFE would have one implementation for such lints and we'd use the same implementation, but ...)

Sure! I can take this on (although I probably won't get around to it/other interop stuff again until Q2),

Great. I'll go ahead and assign you then.

Once we have this auto detection of wasm compatibility on, I suspect (@kevmoo ?) nothing else is needed to turn dart2wasm on by-default in flutter?

@mkustermann
Copy link
Member

@srujzs Above you mentioned you may have time for this in Q2 - any update when this work may happen?

@srujzs
Copy link
Contributor

srujzs commented May 13, 2025

Thanks for the ping! I'm wrapping up on some hot reload-related work - I should be able to tackle this later this month. I assume the plan is to get this in before the next stable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart2wasm Issues for the dart2wasm compiler.
Projects
None yet
Development

No branches or pull requests

5 participants