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

Replace target flags with --target #1369

Merged
merged 1 commit into from
Mar 21, 2019

Conversation

alexcrichton
Copy link
Contributor

This commit deprecates the --web, --no-modules, and --nodejs flags
in favor of one --target flag. The motivation for this commit is to be
consistent between wasm-bindgen and wasm-pack so documentation for
one is applicable for the other (so we don't have to document everywhere
what the translation is between flags). Additionally this should make it
a bit easier to add new targets (if necessary) in the future as it won't
add to the proliferation of flags.

For now the old flags (like --web) continue to be accepted, but
they'll be removed during the next set of breaking changes for
wasm-bindgen.

@alexcrichton
Copy link
Contributor Author

I had this idea when reviewing docs recently and was curious what others though, this is a pretty big change though, so if y'all think an RFC is requested I don't mind doing that.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Beautiful :)

This commit deprecates the `--web`, `--no-modules`, and `--nodejs` flags
in favor of one `--target` flag. The motivation for this commit is to be
consistent between `wasm-bindgen` and `wasm-pack` so documentation for
one is applicable for the other (so we don't have to document everywhere
what the translation is between flags). Additionally this should make it
a bit easier to add new targets (if necessary) in the future as it won't
add to the proliferation of flags.

For now the old flags (like `--web`) continue to be accepted, but
they'll be removed during the next set of breaking changes for
`wasm-bindgen`.
@ashleygwilliams
Copy link
Member

@alexcrichton this is great. we should chat about the deprecation plan as wasm-pack will have to do a bit of dance around how it supports older wasm-bindgen versions (or alternatively we'll need to do a check that the wasm-bindgen version isn't too old.) not to hard but commenting here to flag so i remember to keep an eye on this!

@alexcrichton
Copy link
Contributor Author

I've filed rustwasm/wasm-pack#597 to track this as well

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.

None yet

3 participants