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

Implement the allow-unknown-exports option for the run command. #2879

Merged
merged 1 commit into from
May 11, 2021

Conversation

peterhuene
Copy link
Member

This PR implements the --allow-unknown-exports option to the CLI run
command that will ignore unknown exports in a command module rather than
return an error.

Fixes #2587.

This commit implements the `--allow-unknown-exports` option to the CLI run
command that will ignore unknown exports in a command module rather than
return an error.

Fixes bytecodealliance#2587.
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label May 6, 2021
@github-actions
Copy link

github-actions bot commented May 6, 2021

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@vshymanskyy
Copy link

Wondering, maybe this should be the default behavior?

@tschneidereit
Copy link
Member

Wondering, maybe this should be the default behavior?

As @sunfishcode explained in the linked issue, we don't want this to be the default because it means that these functions pose a risk of becoming de-facto standardized. If modules start being used widely that export functions under names that future standards might want to use, then we'd risk behavior changing in weird and unexpected ways, so we'd likely be forced to choose different names.

This isn't a theoretical concern either: it's the reason why {String, Array}.prototype.includes} isn't called contains, to give just one example.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

Looks good to me. I recognize the practicality of having this option available 😄 .

@peterhuene peterhuene merged commit e36fff8 into bytecodealliance:main May 11, 2021
@peterhuene peterhuene deleted the allow-unknown-exports branch May 11, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wasmtime fails to run a .wasm built with rust-secp256k1, Wasmer can run it
4 participants