Add support for --target web
#567
Conversation
ashleygwilliams
left a comment
ashleygwilliams
left a comment
There was a problem hiding this comment.
this all looks great to me, we just need to make sure to update the docs! the comments in the code would be great to simply copy over into the docs in my opinion. let me know if you want to do that or i can take it on for you.
| } else { | ||
| "--typescript" | ||
| }; | ||
| let target_arg = match target { |
There was a problem hiding this comment.
thanks for making this an enum it's a clear improvement!
| Target::Nodejs => "--nodejs", | ||
| Target::NoModules => "--no-modules", | ||
| Target::Web => "--web", | ||
| Target::Bundler => "--browser", |
There was a problem hiding this comment.
should we perhaps deprecate --browser and add a --bundler alias in wasm-bindgen? (you might already be on this, in which case linking to a PR/issue would be great)
There was a problem hiding this comment.
I think that long term we may want to but for now it serves a purpose we haven't displaced, so it can't be removed just yet
There was a problem hiding this comment.
do we want to open a tracking issue? let me know what you think.
There was a problem hiding this comment.
I've opened up wasm-bindgen/wasm-bindgen#1332 for this
|
rebase to include the panic fix for the tests should make this green! |
|
Sure, I've updated the documentation |
ashleygwilliams
left a comment
ashleygwilliams
left a comment
There was a problem hiding this comment.
one small nit - otherwise i think this is great! thank you! excited to have this ship, i think it's gonna help a bunch of folks!
ashleygwilliams
added
waiting on response
and removed
needs docs
docs/src/commands/build.md
Outdated
| | `nodejs` | Outputs JS that uses CommonJS modules, for use with a `require` statement. `main` key in `package.json`. | | ||
| | `no-modules` | Outputs JS that use no modules. `browser` key in `package.json`. | | ||
| | `browser` | Outputs JS that uses ES6 modules, primarily for use with `import` statements and/or bundlers such as `webpack`. `module` key in `package.json`. `sideEffects: false` by default. | | ||
| | Option | Deployment | Description | |
There was a problem hiding this comment.
hey @alexcrichton sorry for the nit but the word "Deployment" is still used in this chart- could we change it to Usage? thanks!
This commit adds support for the new `--web` flag in `wasm-bindgen` under the flag name `--target web`. To ensure that it was plubmed around the stringly-typed `target` type was switched to an `enum Target` to ensure that all cases it's looked at are handled appropriately for the new `web` target.
ashleygwilliams
left a comment
ashleygwilliams
left a comment
There was a problem hiding this comment.
Thanks so much @alexcrichton - this looks great!
2 participants
This commit adds support for the new
--webflag inwasm-bindgenunder the flag name
--target web. To ensure that it was plubmed aroundthe stringly-typed
targettype was switched to anenum Targettoensure that all cases it's looked at are handled appropriately for the
new
webtarget.