-
Notifications
You must be signed in to change notification settings - Fork 409
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
Possible to publish both CommonJS and ES6 modules in same package? #157
Comments
hey @mbalex99 this is a great question! currently, |
This syntax is wrong: const { greet } = await import("myModule"); It should be like this: import { greet } from "myModule"; I'm not sure if it makes sense to publish CommonJS packages: if Node supports importing to/from wasm, then it should also support ES6 modules. |
hey @Pauan - let's try to be a little more positive and constructive in our comments! yours was a bit blunt and critical (perhaps not your intention!) in a way that i don't think was necessary and doesn't fit the tone i'd like to set for this project. i'd love if you'd edit your comment to be friendlier. simply because we believe something "should" be the case doesn't always mean that it is the case, and we should endeavor to help folks deal with the current restrictions, even if they aren't ideal. responding to your point: Node does not currently support ES6 modules out of the box (they are behind a flag), and many folks definitely still use CommonJS (in fact it's quite common)- i added support for commonJS to |
@ashleygwilliams My intention was certainly not to be rude! How should I point out factually incorrect things in a friendlier way? Ah, I did not realize that wasm-bindgen has support for CommonJS, in that case you're right that it does make sense to support it. |
@ashleygwilliams could you share some of your thoughts on how to do this? |
@Pauan no worries- i didn't think your intention was to be rude at all <3<3 one way to point out things out in a friendly way is to express them as a question e.g. "did you mean to say...?" or to suggest that perhaps it was a mistake. it can also help to make it clear when you are expressing opinions by adding "i think" to statements, e.g. "i think it should be like X" instead if "it should be X". re: how to do this (for @bigfish24 and others), there are 2 strategies i can imagine. 1 is rather brute force, we can just run the tool twice, generating 2 module files and 2 wasm files. i think there's probably a better way, but how wasm-bindgen works right now does require us to do it this way. this is because the wasm file refers to the generated js module file, and i don't think it refers to it in a way that it could refer to two files (tho i may be wrong here). i do know that i've had a few convos with @alexcrichton that suggested that this was not a required configuration (the wasm referring to the js) but just happens to be how it does work right now. i'd want to dig a little deeper into exactly how that works right now and double check my assumptions before moving forward. so yeah, we have a few options:
|
@ashleygwilliams thanks for the insight. Right now our project is constructing 2 js and 2 wasm. Which is fine but makes building a library on top sort of force their project to build 2 different paths. Ideally the 2nd way sounds much simpler for the future. Thanks! CommonJS is sort of the standard when it comes to publishing it on NPM. |
I think I may have a solution in a PR to Any comments on the implementation there would be greatly appreciated. If this gets merged we could then add a new |
It seems that this discussion is redundant with the issue rustwasm/wasm-bindgen#233, see rustwasm/wasm-bindgen#233 (comment). Here's what I have in mind: rustwasm/wasm-bindgen#326 (comment) |
tracking progress on this in #313 |
It looks like wasm_pack is sort of one or the other when it comes to publishing something for webpack browser consumption and one for nodejs.
Is it possible that wasm_pack can publish a package for both at the same time?
Ideally something like this:
Node
Browser in Webpack
The text was updated successfully, but these errors were encountered: