Publish ESM packages following ESM spec

[JamieDanielson]

Is your feature request related to a problem? Please describe.

Our packages currently publish directories of ESM builds, but they do not follow ESM spec and therefore cause problems for some end users trying to use them. For example, see issue #3989 .

Required (for each package) to fully support ESM

• Fix every relative import and export to be ./fileName.js (may require adjustments to imports/exports that import directories)
• Change the package type to module in package.json
• Add exports section to package.json (*, import, require)
• Add tsconfig.esm.json if it doesn't already exist
• Add separate build scripts for cjs and esm, along with command line addition of type:module and type:commonjs in a package.json
• Rename .eslintrc.js to .eslintrc.cjs (or change to ESM syntax instead of using module.exports)
• Update .tsconfigs to parse .cjs extensions
• Update mocha to use ts-node/esm loader (also requires install of ts-node)

Maybe Required, otherwise Recommended

• Upgrade TypeScript to at least 4.7 to allow using nodenext in tsconfig.esm.json, which integrates with node's native ESM support and gives warnings for missing .js suffixes.
• Adjust tests using sinon.spy or sinon.stub because you can't spy or stub ES modules
• update the test command to use tsconfig.esm.json
• add a test command that builds the package and runs mocha against the built cjs (instead of ts-mocha against the ts source code)... idea from this blog
• Add (or update?) smoke / integration tests
• Consider a tool like arethetypeswrong or publint to verify types

This (closed) PR can be used as a reference.

I would argue that we need a testing strategy in place to verify these changes, BEFORE we make any changes, as our current ESM tests cover a lot of ground but not the problems solved with this change. There are a few issues floating around I believe, but for now I will link #4008 to build out smoke / integration tests.

Related Issues that may be resolved with these changes:

• 3989

• 4106

• 4392

• 4396

• 4642

• 4794

• 4842

[david-luna]

Hi @JamieDanielson

That's a pretty detailed list. Thanks! :) Is it a proposal or already has a consensus with other maintainers?

[trentm]

• Change the package type to module in package.json

We don't need this step do we? If "exports" in package.json has the necessary "import" entries, then leaving empty or explicitly "type": "commonjs" should be fine.

[JamieDanielson]

• Change the package type to module in package.json

We don't need this step do we? If "exports" in package.json has the necessary "import" entries, then leaving empty or explicitly "type": "commonjs" should be fine.

I have to research more to confirm. I'd read something about either how the code is transpiled or how the types are generated that require type: module but offhand I don't quite remember. All the examples I've seen of folks doing this approach, they updated the type. But as time goes on and more features come out to help with interop, I can see some of these requirements potentially going away (aside from minimum TypeScript and Node versions to take advantage of said features). When we're ready to move forward with this (will likely want more testing in place first) then we can revisit whether this change is needed.

[ianwremmel]

This is all anecdotal; I've been trying to figure out how to dual-publish for a while and I imagine there are multiple "correct" approaches. A big part of the struggle comes from TypeScripts's behaviors not being entirely documented (like, if we just wrote typesafe mjs and eliminated the typescript compile step, we pretty wouldn't every have folks complaining about cjs/esm issues). In any case, this is what I believe has to be done to dual publish (at least as far as package.json and build outputs):

• produce separate output dirs with .cjs files and .mjs files and set exports accordingly (this means the package.json module field doesn't matter to node's resolver)
• produce separate separate .d.cts and .d.mts type definitions and set exports accordingly. In particular, you need separate types entries for both import and require. (i still apparently don't have this quite right as I only discovered publint recently; it's telling me i should be using .d.cts where i'm currently using .d.ts)
• set "type": "module" in package.json anyway, otherwise tsc in the consuming project will typecheck against the cts versions, which is probably not what you want. (this may be related to my above comment about .d.ts vs .d.cts, but I don't think so).

So, this is a really long way to say, "yes, i think the type in package.json remains necessary".

If it's helpful, I've got this repo seemingly dual-publishing correctly (aside from the .d.ts vs .d.cts issue).