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.

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

Experimental wasm support #1777

Merged
merged 23 commits into from
Oct 27, 2022
Merged

Experimental wasm support #1777

merged 23 commits into from
Oct 27, 2022

Conversation

jakemac53
Copy link
Contributor

  • Adds the experimental platform experimental-chrome-wasm. This requires chrome beta and a very recent SDK to run.
    • This platform will be deleted eventually, and replaced with a --compiler flag.
  • Adds wasm to the boolean selector variables, and an isWasm field to Runtime (defaults to false, so should be non-breaking).

Note that the integration test for this will initially be failing, as there is one more change needed from the SDK, but once that lands we can update this to point at the new location for the wasm runtime, update the min SDK, re-run the tests, and land.

@jakemac53 jakemac53 mentioned this pull request Oct 20, 2022
@jakemac53
Copy link
Contributor Author

cc @christopherfujino note that this isn't the final API we want here, but if you want to start thinking about trying to get flutter tests working with dart2wasm, then this should provide the basic building blocks required. Just know that the api is highly unstable and subject to change.

@jakemac53
Copy link
Contributor Author

@natebosch not sure how you feel about this runtime showing up in the help text - we could add some logic in to hide any runtime with experimental in the name, or add a boolean hidden field to runtime, in order to hide it. But also I don't think it hurts to have it visible, given the name clearly indicates the status of it.

@natebosch
Copy link
Member

@natebosch not sure how you feel about this runtime showing up in the help text - we could add some logic in to hide any runtime with experimental in the name, or add a boolean hidden field to runtime, in order to hide it. But also I don't think it hurts to have it visible, given the name clearly indicates the status of it.

I'd be OK with either filtering out names that contain "experimental", or with leaving it in. @joshualitt - is there any concern about the feature "leaking", or having folks notice it and try it out while we are still getting it nailed down in the SDK?

@joshualitt
Copy link
Contributor

@natebosch not sure how you feel about this runtime showing up in the help text - we could add some logic in to hide any runtime with experimental in the name, or add a boolean hidden field to runtime, in order to hide it. But also I don't think it hurts to have it visible, given the name clearly indicates the status of it.

I'd be OK with either filtering out names that contain "experimental", or with leaving it in. @joshualitt - is there any concern about the feature "leaking", or having folks notice it and try it out while we are still getting it nailed down in the SDK?

Great question. I think this should be just fine because the flag is self descriptive and has to be explicitly passed. There is no danger here of someone 'accidentally' switching to Wasm and being very confused.

As to the broader question, as I understand things until we add support for Dart2Wasm to the CLI, we're not officially advertising Dart2Wasm as supported. It's true that adding support to package:test does lower the bar a bit to entry, but only for writing tests, which seems pretty harmless to me. Specifically, there is no danger here of anyone shipping something with Dart2Wasm before we are ready because of this work, so I think it should be completely safe.

@jakemac53 jakemac53 merged commit b82fc0b into master Oct 27, 2022
@jakemac53 jakemac53 deleted the wasm branch October 27, 2022 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants