-
Notifications
You must be signed in to change notification settings - Fork 912
Open
Labels
needs:refinementThis issue needs to be refined/broken apart into sub-issues before implementationThis issue needs to be refined/broken apart into sub-issues before implementationnever-staletype:feature-trackingA feature with sub-issues that need to be addressedA feature with sub-issues that need to be addressed
Description
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
inpackage.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
andtype:commonjs
in apackage.json
- Rename
.eslintrc.js
to.eslintrc.cjs
(or change to ESM syntax instead of usingmodule.exports
) - Update
.tsconfig
s to parse.cjs
extensions - Update mocha to use
ts-node/esm
loader (also requires install ofts-node
)
Maybe Required, otherwise Recommended
- Upgrade TypeScript to at least 4.7 to allow using
nodenext
intsconfig.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:
- ESM output not functional #3989
- Suggestion: Use
.ts
extensions for internal imports/exports #4106 - Provide working TypeScript / ESM examples #4392
- NodeJS/ESM Compliance - use import/export file extensions #4396
require
calls inesm
build of@opentelemetry/resources
#4642- Wrong import for "OTLPExporterNodeBase" #4794
- Can not use ESM support in lambda environment #4842
ianwremmel, jaydenseric, chicoxyzzy, RaphaelManke, michalkvasnicak and 2 more
Metadata
Metadata
Assignees
Labels
needs:refinementThis issue needs to be refined/broken apart into sub-issues before implementationThis issue needs to be refined/broken apart into sub-issues before implementationnever-staletype:feature-trackingA feature with sub-issues that need to be addressedA feature with sub-issues that need to be addressed