Add node module output format

[Systemcluster]

This PR adds a node module output target. Node stabilized node modules with release 12, a lot of the JS ecosystem moved towards publishing modules, and the other targets apart from nodejs all generate modules.

There were already code-paths for this target, but it wasn't hooked up or finished. I changed the naming away from "experimental" and finished the implementation.

Support for the format would also need to be added to wasm-pack after this is merged.

[daxpedda]

Even though the changes are very small, personally I can't review this as I have no clue about Node.

Best case scenario I can take a look at it later when I have more time to learn whats needed here, but even then I would keep it experimental until a maintainer who knows whats going on here can review this.

[Systemcluster]

Thanks for checking, I'll annotate some of the changes later to make it easier to review.

[Systemcluster]

I've split up the changes into separate commits, so it should be more obvious what changes what. The core changes are in 4d63628. In general, the new target mostly uses the bundler target paths, plus the existing node import logic as noted below.

[Systemcluster]

My earlier question was resolved after gaining some more familiarity with the codebase, I've updated my comments where I was being unclear.

@daxpedda I think you should be able to review this in this state.

[daxpedda]

LGTM!

I'm happy to move this forward with the following changes:

• Name the option experimental-nodejs-module.
  • I don't feel entirely comfortable otherwise, considering my lack of knowledge with this tech.
  • After some digging I do believe we want to produce .mjs files, so until we figure that out we shouldn't stabilize the option.
• Make sure its being tested in CI.
  The CI is a bit of a mess and I have no guidance on where to add it, unfortunately you will have to dig into it yourself.

[Systemcluster]

I don't feel entirely comfortable otherwise, considering my lack of knowledge with this tech.

I'm fine with experimental as long as it's merged. 🙂

After some digging I do believe we want to produce .mjs files, so until we figure that out we shouldn't stabilize the option.

That will require a few more changes than 4d63628 if I've seen right, so far I was trying to change as little as possible. If I go through and generalize the extension already, I'll also add a command line option for overwriting the default.

Just to make clear, currently there are the following targets:

Target	Type	Extension
bundler, deno, web	ESM	.js
nodejs	CJS	.js
no-modules	Integrated	.js

Changing it for this target would add another row:

Target	Type	Extension
nodejs-module	ESM	.mjs

While I don't disagree that a .mjs extension would make sense for the target itself, it would be surprising with the other targets being as they are. Consider if an option to manually set the extension and leaving the default consistent with the others it isn't better here at this point.

In either case, since this remains experimental for now we can do that in a follow-up PR.

The CI is a bit of a mess and I have no guidance on where to add it, unfortunately you will have to dig into it yourself.

I didn't find any relevant tests on first glance and was hoping for more info, but I'll take digging in. 😄

[daxpedda]

While I don't disagree that a .mjs extension would make sense for the target itself, it would be surprising with the other targets being as they are. Consider if an option to manually set the extension and leaving the default consistent with the others it isn't better here at this point.

Is there a reason file extensions should be consistent across targets?
I certainly would not have expected so, because targets are in no way equal, they have to be handled very differently.

In either case, since this remains experimental for now we can do that in a follow-up PR.

Agreed.

---

Your work and input so far is really appreciated, it has been very valuable and you put a lot of effort into making this as easy to review as possible.

Thank you!

[Systemcluster]

Is there a reason file extensions should be consistent across targets?

Principle of least surprise is the main one, but I don't have reasons beyond that.

because targets are in no way equal, they have to be handled very differently.

Technically yes, but only to some degree. If you want to distribute your wasm-bindgen generated files on NPM you always have to package it with a package.json, and resolution through package.json by default goes through Node. TypeScript and other tools¹ also adhere to the Node resolution strategy, either by default or as an option, and that resolution will consider the "type" field in the package.json to decide whether .js files are ESM or CJS. That part is the same with all targets.

I'll think about it more until the follow-up. I actually don't mind using .mjs here, I just wish we could make the nodejs target output .cjs instead (or both).

¹With exceptions, Webpack states it autodetects the format, some other bundlers might do the same.

[Systemcluster]

It looks like for use-cases without a package.json the explicit extension is really useful 😄

Test is running now, I added a package.json temporarily to make the test work. It can be removed with the extension change.

[daxpedda]

Great work, thank you!

[Systemcluster]

It took a bit of back and forth but we got there, thanks for the review and guidance!

[magic-akari]

I think it might be necessary to mention #3790

[Systemcluster]

Thanks for linking that issue, that's a feature I want too. I wanted to experiment with some approaches during the week.