Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move dart2wasm support to be "normal" #2143

Closed
3 tasks
kevmoo opened this issue Nov 28, 2023 · 0 comments · Fixed by #2144
Closed
3 tasks

Move dart2wasm support to be "normal" #2143

kevmoo opened this issue Nov 28, 2023 · 0 comments · Fixed by #2144
Assignees
@jakemac53
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@kevmoo
Copy link
Member

kevmoo commented Nov 28, 2023 •
edited

At the moment there is an experimental-chrome-wasm flag, etc.

Ideally, Wasm support should just be like dart2js support. Wasm tests should "just pass" on (latest) Chrome and Firefox.

We need to test the following packages with wasm

CC @jakemac53 @srujzs
@kevmoo kevmoo added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Nov 28, 2023
This was referenced Nov 28, 2023
Open
Open
Open
@jakemac53 jakemac53 self-assigned this Nov 30, 2023
@jakemac53 jakemac53 mentioned this issue Nov 30, 2023
Merged
auto-submit bot pushed a commit that referenced this issue Dec 5, 2023
@jakemac53
Promote dart2wasm compiler support to the stable browser platform. (#…
e49ae54 
…2144)

Closes #2143

I ended up going on quite the cleanup adventure here � . But I think the result is much cleaner around how it handles --precompiled and the various compilers in the browser platform. We may want to adopt a similar cleanup in other platforms too.

- Deletes the experimental wasm platform, moves it into the `chrome` platform (works on chrome stable now).
- Refactors heavily how the browser platform works, abstracting away the logic for precompiled support, dart2js support, and dart2wasm support into separate classes which implement a new `CompilerSupport` interface.
- Drops entirely support for `--pub-serve` mode. We could bring this back if necessary as another kind of `CompilerSupport` but I don't think it has any users (let me know if you feel different).
- Each `CompilerSupport` instance now gets its own server, browser manager, secret, etc. This resolves complications around knowing which kinds of compiler a given request is asking for, which the previous model couldn't handle well. You will get multiple browser instances if multiple compilers are used.
- ~Adds the "precompiled" compiler option. This isn't intended to be passed directly, it is inferred when passing the `--precompiled` argument. No other compiler options are allowed if it is used (although we could probably if desired?).~
- Dropped `isJS` and `isWasm` from `Runtime`, these no longer make sense. Added extensions to `Compiler` instead for convenience.

Likely there is some additional consolidation that could happen here, as there is a fair bit of duplicate logic across some of the implementations. But I am not sure if it is really worth it or not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jakemac53 jakemac53

Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Promote dart2wasm compiler support to the stable browser platform. dart-lang/test
2 participants
@kevmoo @jakemac53