Include *_bg.js wasm-bindgen outputs in package.json #200
Conversation
src/manifest.rs
Outdated
| @@ -72,6 +72,8 @@ impl CargoManifest { | |||
| let filename = self.package.name.replace("-", "_"); | |||
| let wasm_file = format!("{}_bg.wasm", filename); | |||
| let js_file = format!("{}.js", filename); | |||
There was a problem hiding this comment.
i think we can remove this now!
There was a problem hiding this comment.
Just wanna double-check this one. It looks like removing this would break an existing test (it_creates_a_package_json_default_path(), line 63 in the file). Do we not want this js_file to become the "main" attribute in package.json?
|
hey @mciantyre i am SO sorry to do this, but i had totally missed #197 which also fixes this issue. if you are interested in making the tests more robust as you already have... if you could rebase, i would definitely still accept this PR as a test improvement. sorry again about the confusion and misleading comments :/ additionally- if there's any other issue you'd like to grab, i am happy to reserve one for you for the trouble with this (also happy to mentor an issue if you'd like) |
|
No problem! I rebased. Let me know how this looks. |
| @@ -79,6 +84,17 @@ fn it_creates_a_package_json_provided_path() { | |||
| assert!(utils::read_package_json(&path).is_ok()); | |||
| let pkg = utils::read_package_json(&path).unwrap(); | |||
| assert_eq!(pkg.name, "js-hello-world"); | |||
| assert_eq!(pkg.main, "js_hello_world.js"); | |||
There was a problem hiding this comment.
hrm, this seems to assert that the main is not the bg file.... that's not correct right? i might pull this down and just figure out what's happening here.
There was a problem hiding this comment.
sorry for the confusion around this. i'm honestly baffled about how this bug happened but feels like something i might just need to sort out. thank you so much for all your patience and hard work. i'll pull this down and take it from here so you don't have to worry about it <3<3
|
closing in favor of #205 which has these commits <3 thank you again! |
Fixes #199!
As @ashleygwilliams suggested, I updated the manifest module to add the
*_bg.js-qualified output inpackage.json'sfilesattribute. I updated existing tests to expect the new file. I added a few other asserts in existing tests to expect the files inpackage.json.I came upon this issue after taking a
wasm-pack packoutput and using it in a new node project. Would we want an automated test to make sure that use-case always works? I'm not sure what that kind of test might look like, but if we feel its valuable I'd be happy to dig deeper!I haven't looked into how this issue came to be. Did something change in
wasm-bindgen; would our solution here be sufficient for any future changes? If there's a need, I'd be happy to help build a more resilient fix.Thanks for all of your help and effort in building
wasm-pack!Make sure these boxes are checked! 📦✅
rustfmtinstalled and have yourcloned directory set to nightly
$ rustup override set nightly $ rustup component add rustfmt-preview --toolchain nightlyrustfmton the code base before submitting✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨