Support compiling to wasm in build_web_compilers

[simolus3]

Add dart2wasm as a compiler option for build_web_compilers.

This is still incomplete, since:

• We need to wait for dart compile wasm to support --multi-root: https://dart-review.googlesource.com/c/sdk/+/377662
• It's not clear if we want to let users customize the module loader, e.g. by injecting additional imports or wasm files. That would probably be hard to pull off with the existing implementation.
• This should support custom defines / compiler options.
• It obviously needs integration tests.

Closes #3621

[jakemac53]

One higher level comment here, is that we want to be able to support some form of auto-switching between dart2wasm and dart2js based on browser support.

This would imply being able to compile both with dart2js and with dart2wasm simultaneously, which somewhat complicates things.

[simolus3]

This would imply being able to compile both with dart2js and with dart2wasm simultaneously, which somewhat complicates things.

I actually ran into this problem already, haha. I thought I could just apply the entrypoint builder twice to different targets to fix this, but build_runner doesn't like that due to conflicting outputs.

I think in the end we may want to support four options in the compiler argument then?

1. dartdevc - as is
2. dart2js - as is
3. dart2wasm - new option, only compile with dart compile wasm
4. both (ideally the new default) - compile with dart2js and dart2wasm.

For 4, the question is how the auto-switching would look like. We could embed a suffix to the dart2js entrypoint file to either invoke the dartProgram directly or instead load the wasm file and use that. That uses fewer files, but requires users to download the potentially large dart2js file.
Or we could change the dart2jsBootstrap logic to be aware of a wasm build happening as well, and then emit say a .dart2js.js file instead. Then we generate a small additional .js entrypoint file starting a feature detection and loading the appropriate file? From a quick look that appears to be what Flutter is doing.

[kevmoo]

I support getting wasm working first, then enabling the runtime detection. We should look to see what Flutter does here. I wish we had a good place to put the shared logic!

[jakemac53]

I think in the end we may want to support four options in the compiler argument then?

We may need dart2wasm+DDC and dart2wasm+dart2js eventually?

I was thinking about splitting out whether "wasm" is enabled or not as a separate thing? Then we don't have to have an option for every combination. Maybe, an enable_wasm boolean option, instead of making it a part of the compiler option. And then people can set the compiler separately from whether or not they enable wasm. Possibly a new "none" compiler option for just wasm. It would make more sense if "compiler" was "js_compiler" or something though.

Or we could change the dart2jsBootstrap logic to be aware of a wasm build happening as well, and then emit say a .dart2js.js file instead. Then we generate a small additional .js entrypoint file starting a feature detection and loading the appropriate file? From a quick look that appears to be what Flutter is doing.

Yeah I think we need something like this.

I support getting wasm working first, then enabling the runtime detection. We should look to see what Flutter does here. I wish we had a good place to put the shared logic!

I am fine with that, but we need to make sure the user facing configuration is compatible with a future mode that does runtime detection.

[simolus3]

We may need dart2wasm+DDC and dart2wasm+dart2js eventually?

I agree that eventually compiling with both + emitting a small launcher doing feature detection and picking the appropriate one is the way to go. It's also a good default.

However, there are also some cases where we want to compile with both, but also make an explicit decision about which one to use. One case that comes to mind is prebuilt tests with build_test when passing --compiler dart2js --compiler dart2wasm. We really need both to be compiled by build_web_compilers for that, but the test runner needs some way of then loading the right one. Given that prebuilt dart2wasm appears to be supported by package:test already even though it's not supported by build_web_compilers, I assume there's another tool somewhere generating prebuilt wasm tests? What are the outputs required for that setup?

[jakemac53]

iven that prebuilt dart2wasm appears to be supported by package:test already even though it's not supported by build_web_compilers, I assume there's another tool somewhere generating prebuilt wasm tests?

I don't believe there is one, the SDK does completely its own thing for wasm testing (I have no idea how this works).

[simolus3]

I think this PR now covers everything for basic dart2wasm support - I've opened #3730 for the future work of compiling with multiple compilers.

@jakemac53 I've added you as a reviewer on https://dart-review.googlesource.com/c/sdk/+/377662 which needs to land before we can ship this. I don't what the right process is to get SDK CLs reviewed, could you help me find reviewers for dart2wasm that may review that CL?

[jakemac53]

This could just be the default import probably too, but either way works. @kevmoo do we have a recommended way to target wasm with conditional imports?

[kevmoo]

NO! @sigmundch is working on that RIGHT NOW.

See dart-lang/sdk@e8a049f

After this lands we can check for js_interop & !js

[kevmoo]

(er, after we have a dev SDK that supports this release)

[simolus3]

I've updated the import chain to check for dart:js not being available and dart:js_interop being available.

However, citing the 3.5 release announcement:

We plan on deprecating the old interop APIs (dart:html, dart:js, package:js, etc.) in our next Dart release, and fully discontinuing them later next year.

This makes it sound like there's soon going to be a dart2js version that doesn't support dart:js either, which will once again mean that there's no way to distinguish the compilers through conditional imports.

[kevmoo]

@simolus3 – if we wait for 3.6.0-150.0.dev, that should have dart-lang/sdk@ca77bea

We'll just need to ponder if we want build_web_compilers limited to that SDK for a while. Something to discuss...

[jakemac53]

Fwiw I am perfectly fine with build_web_compilers only supporting dev SDKs or whatever, it is often in that state. And nobody seems to complain 🤷‍♂️

[kevmoo]

@jakemac53 – ditto!

[kevmoo]

Ooo! Looks like 3.6.0-150.0.dev has the fix you want.

So
dart:io -> vm only
dart:js -> JS only
otherwise, wasm

[kevmoo]

@simolus3 – for auto-switch, have you looked at the code in Flutter?

[simolus3]

@simolus3 – for auto-switch, have you looked at the code in Flutter?

Flutter is doing a lot more (like also importing skwasm and so on), but the loader script is minified from these sources using esbuild. I think in the end we can pretty much copy this check to detect whether the browser supports dart2wasm. It's unlikely that we can ever share the files though.

Is it fine if we do this in another PR?

[kevmoo]

Need to do some work to be CI happy...

[simolus3]

The latest release on the dev channel is still 3.6.0-149.0.dev, so it looks like we'll have to wait for that to get updated first.

[github-actions]

PR Health

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

Package publish validation ✔️

Package	Version	Status
package:build	2.4.2-wip	WIP (no publish necessary)
package:build_config	1.1.2-wip	WIP (no publish necessary)
package:build_daemon	4.0.3-wip	WIP (no publish necessary)
package:build_modules	5.0.10-wip	WIP (no publish necessary)
package:build_resolvers	2.4.3-wip	WIP (no publish necessary)
package:build_runner	2.4.13-wip	WIP (no publish necessary)
package:build_runner_core	7.3.3-wip	WIP (no publish necessary)
package:build_test	2.2.3-wip	WIP (no publish necessary)
package:build_web_compilers	4.0.12-wip	WIP (no publish necessary)
package:scratch_space	1.0.3-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[kevmoo]

I guess we DON'T have a release after -149. 🤷

[simolus3]

Looks like 3.6.0-165.0-dev has just been released, so maybe it works when triggering the CI again?

[simolus3]

Fwiw, one option is to remove build_web_compilers from the workspace for a bit. That would also be somewhat unfortunate, but maybe better in the long term.

I've done that, let's see if it works. A downside is that _test can't be part of the workspace either then (because it needs a dependency override to the path build_web_compilers, and dependency overrides are shared in workspaces).

[kevmoo]

Thanks for your patience here @simolus3 – lets see if tests pass

[sigurdm]

Yeah I am not concerned about requiring 3.5 (for workspace support generally). It is more the requirement of all packages having the same min SDK in order to be tested on their min SDK that is unfortunate (since it isn't a one time thing, that will be a persistent issue).

Not sure I follow - that is a feature request for mono_repo?

Maybe it would be possible to pass a flag to pub upgrade to tell it to only solve for the current package, even if it is in a workspace, including respecting any dependency overrides in the workspace pubspec and implicitly using path dependencies for other packages in the repo? That sounds like kind of a complicated feature potentially though, but it is essentially what is needed here.

I think this is the same request as dart-lang/pub#4357

[simolus3]

Let me know if there's anything else for me to do here now that the tests are passing. I think the package version thing is a mono_repo issue primarily (it tests each package with the oldest SDK it claims to support, which can break in workspaces if other packages require newer SDKs).
I've added the workaround of removing build_web_compilers from the workspace in the meantime, I think we can revert that change with the 3.6.0 stable release.

[jakemac53]

I think this is the same request as dart-lang/pub#4357

Similar, but not quite. The main difference is I would still want it to use a path dependency for all the packages in the workspace that the package does depend on. I do have access to the entire workspace, and not just a single package, and the package may rely on unpublished functionality from some of the workspace packages.

[jakemac53]

Just took a final pass with a few final nits, then we can land this. @kevmoo would we want to ship this ASAP?

[sigurdm]

Similar, but not quite. The main difference is I would still want it to use a path dependency for all the packages in the workspace that the package does depend on. I do have access to the entire workspace, and not just a single package, and the package may rely on unpublished functionality from some of the workspace packages.

Not sure I follow - if you feel like it, could you open a feature request with perhaps an example scenario of what you would like to be able to do?