Skip to content

feat(build): add first-class support for binary crates#736

Open
boringcactus wants to merge 5 commits intowasm-bindgen:masterfrom
boringcactus:first-class-bins
Open

feat(build): add first-class support for binary crates#736
boringcactus wants to merge 5 commits intowasm-bindgen:masterfrom
boringcactus:first-class-bins

Conversation

@boringcactus
Copy link
Copy Markdown

Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed
$ rustup component add rustfmt
  • You ran cargo fmt on the code base before submitting
  • You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨


fixes #734

Now that wasm-bindgen/wasm-bindgen#1630 has been implemented, I think it's good to have the same support in wasm-pack.

This implementation currently doesn't add bin entries in package.json, partially because the docs on that field say that targets must be prefixed with #!/usr/bin/env node and I'm not sure how or where to ensure that all and only binaries get that prefix. I don't think that feature is vital, but if other people think it's a must-have then I can take a look at adding it.

bors bot added a commit to grovesNL/glow that referenced this pull request Nov 22, 2019
61: Instructions for the stdweb backend in the hello example. r=grovesNL a=theypsilon

Previously there weren't instructions to run the 'hello' example on stdweb, so I'm adding them.

But I also noticed that the README.md is not quite right yet. The web-sys backend instructions seem obsolete, but I can't find and easy way to make it work with wasm-pack because 'hello' is a binary crate.

Here is a pull request to make wasm-pack compatible with binary crates: wasm-bindgen/wasm-pack#736

Co-authored-by: José manuel Barroso Galindo <theypsilon@gmail.com>
@boringcactus
Copy link
Copy Markdown
Author

currently the way to get examples built is to throw --bins --examples straight to cargo build. should we accept a --example foo argument instead? that's probably cleaner but also a little bit more work on the implementation side

@boringcactus
Copy link
Copy Markdown
Author

--example foo now works.

@boringcactus
Copy link
Copy Markdown
Author

boringcactus commented Jan 11, 2020

issue with the original draft of this PR: if a binary is defined but required features aren't available, it doesn't get built, but this code still tries to run wasm-bindgen against it, causing problems. i fixed it, though.

this handles, e.g., binaries missing required features
@ashleygwilliams ashleygwilliams modified the milestones: 0.9.0, 0.10.0 Jan 31, 2020
Copy link
Copy Markdown
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Honestly I can't find anything to complain about here, I also tried it locally, seems to work perfectly.
As mentioned in my review, I think the PR can be kept simpler if we don't try to handle compiling multiple packages at once here and leave that to #732.

Is there anything else that can be done to move this forward?
@boringcactus could I give you a hand rebasing this?

Comment thread src/bindgen.rs
Comment thread src/manifest/mod.rs
@alvinhochun
Copy link
Copy Markdown

Other than a rebase (which #816 tried to do), is there anything else that is blocking the merge of this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support building binary crates

4 participants