opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Publish ESM packages following ESM spec

Open JamieDanielson opened this issue 1 year ago • 2 comments
trafficstars

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

JamieDanielson avatar Aug 01 '24 18:08 JamieDanielson