build command with --out-name that contains dots, `.`, generates incorrect files

[torch2424]

🐛 Bug description

When creating a PR for #313, and is a bit of a blocker if we want to follow the . semantic of how JS is usually output for libraries 😞 , I noticed that the --out-name flag does not work properly if it contains a . character. I assume this is also an issue with wasm-bindgen, but it would also affect what we do here.

I think this is better done by showing in examples. I'll use the node target since it requires the _bg.js shim, which also doesn't generate correctly.

Examples:

Happy Case:

Expected output

Case where name contains *.js:

• Correctly ignores the .js extension when generating the main files.
• Missing shim
• wasm file is incorrectly names
• package json references non-existant files

Case where name contains *.somerandomextension:

• Generated files ignore the extension
• Package.json references the correctly names files, but they were generated incorrectly

Case where name contains *.*.js:

• Correctly ignores the .js extension when generating the main files.
• Missing shim
• wasm file is incorrectly names
• package json references non-existant files

🤔 Expected Behavior

Whenever there is a . in the --out-name, it should respect the additional extensions, and then build off of them. There probably is some discussion points of:

• How should the _bg be handled?
• Should .js or .wasm be ignored?
• Do we only want to solve this on the wasm-bindgen layer, and then not allow the extensions on the wasm-pack layer?

👟 Steps to reproduce

Please see the examples screenshots 😄

🌍 Your environment

Include the relevant details of your environment.
wasm-pack version: current master
rustc version: 1.34.1

pop_OS! is a derivative of Ubuntu 😄

Thank you! 😄

cc @ashleygwilliams @alexcrichton @ibaryshnikov (since @ibaryshnikov opened #599 , and I see ya'll in the rust/wasm WG meetings 😄 )

[torch2424]

So I thought about it, so what I could do, do hack around this is:

1. Generate the wasm-bindgen output with an out-name with no ..
2. Use fs to rename
3. repeats steps 1-2 for every different target

But then we'll have to make sure to coordinate this with wasm-bindgen when it is fixed, because then we'd still have the same issues with adding the extra .js I think 🤔

[alexcrichton]

Thanks for the report! I think I may be a bit confused though, can you clarify what the expected behavior you're thinking we should do is? For example with --out-name foo.js, what should wasm-bindgen be generating and what would wasm-pack be pointing to?

[torch2424]

@alexcrichton

You are welcome! 😄

Well, the behavior can really go two ways. For this example, let's pretend we have the flags --out-name index.cjs.js --target nodejs:

1. We do a strip and suffix of .js in the out name, thus, making it purely index.cjs. Thus, ideally, we would have the output:

Generated files

package.json
README.md
.gitignore
index.cjs.d.ts
index.cjs.js
index_bg.cjs.js
index_bg.cjs.wasm

package.json

{
 "//": "Other stuff here",
"files": [
"package.json"
"README.md"
".gitignore"
"index.cjs.d.ts"
"index.cjs.ts"
"index_bg.cjs.js"
"index_bg.cjs.wasm"
]
}

Please note here, that index.cjs became index_bg.cjs.js, and not index.cjs_bg.js. This is because .cjs.js usually means this is a common js, JS module. Though, that is if we want to follow the "standard".

2. We ignore .js, and always append another .js

Generated files

"package.json"
"README.md"
".gitignore"
"index.cjs.js.d.ts"
"index.cjs.js.js"
"index_bg.cjs.js.js"
"index_bg.cjs.js.wasm"

package.json

{
 "//": "Other stuff here",
"files": [
"package.json"
"README.md"
".gitignore"
"index.cjs.js.d.ts"
"index.cjs.js.js"
"index_bg.cjs.js.js"
"index_bg.cjs.js.wasm"
]
}

3. We strip all . before before passing to wasm-bindgen, and then do renaming ourselves to re-prepend anything after the first .. (Which honestly unblocks my --target all PR, and helps follow the "standard naming" conventions. But ideally this would be fixed on the dependency, and then us.)

Generated files

package.json
README.md
.gitignore
index.cjs.js.d.ts
index.cjs.js.js
index_bg.cjs.js.js
index_bg.cjs.js.wasm

package.json

{
"//":  "Other stuff here",
"files": [
"package.json"
"README.md"
".gitignore"
"index.cjs.js.d.ts"
"index.cjs.js.js"
"index_bg.cjs.js.js"
"index_bg.cjs.js.wasm"
]
}

Let me know what you think! Thanks! 😄

[alexcrichton]

I think I'd find it a little odd if we stripped off .js at the end and just ignored it, it seems like callers could otherwise just strip the .js themselves. How important is it to follow conventions here? It seems sort of odd to mangle the base name to have a desired output of index_bg.cjs.wasm as opposed to something like index_cjs_bg.wasm or something like that.

I'm also curious what following these conventions would enable? I'm assuming that the underlying tools consuming these files don't really care about the names, but I may be missing some other tools which interoperate with these conventions.

[torch2424]

@alexcrichton

Sorry for the late reply! Been super busy with work, and a Jazz festival over the weekend 😂

I think I'd find it a little odd if we stripped off .js at the end and just ignored it, it seems like callers could otherwise just strip the .js themselves.

I agree here 👍 So in the case of --out-name index.js we'd generate index.js.js correct?

How important is it to follow conventions here? It seems sort of odd to mangle the base name to have a desired output of index_bg.cjs.wasm as opposed to something like index_cjs_bg.wasm or something like that.

Personally I try to follow conventions where I can. As, it tends to make things more maintainable, and healthier for the ecosystem. If we are trying to meet JavaScript / node developers where they are, than I think conventions are important. Otherwise, I'd be open to do index_cjs_bg.wasm.

I'm also curious what following these conventions would enable? I'm assuming that the underlying tools consuming these files don't really care about the names, but I may be missing some other tools which interoperate with these conventions.

From my understanding, most tools do not rely on file naming for their bundling and things. So yes, we should be fine not following conventions in terms of using the tools themselves 😄

[alexcrichton]

Yeah so I think we'll definitely want to avoid just unconditionally stripping .js, but for conventions I agree that it's good to follow them but this is all generated code that isn't really inspected that much so I think it's fine if we aren't 100% conventional, even in the generated code. In that sense if index_cjs_bg.wasm works I think it's probably best to take that route for now.

[torch2424]

@alexcrichton Sounds good, I'll go forward with this for now! 🎉 But I'll keep the issue open as the problem will probably still need to be adressed down the line for people other than me 😄