-
Notifications
You must be signed in to change notification settings - Fork 833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Publish ESM packages following ESM spec #4898
Comments
That's a pretty detailed list. Thanks! :) Is it a proposal or already has a consensus with other maintainers? |
We don't need this step do we? If "exports" in package.json has the necessary "import" entries, then leaving empty or explicitly |
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 |
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
So, this is a really long way to say, "yes, i think the If it's helpful, I've got this repo seemingly dual-publishing correctly (aside from the |
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
./fileName.js
(may require adjustments to imports/exports that import directories)module
inpackage.json
package.json
(*, import, require
)tsconfig.esm.json
if it doesn't already existtype:module
andtype:commonjs
in apackage.json
.eslintrc.js
to.eslintrc.cjs
(or change to ESM syntax instead of usingmodule.exports
).tsconfig
s to parse.cjs
extensionsts-node/esm
loader (also requires install ofts-node
)Maybe Required, otherwise Recommended
nodenext
intsconfig.esm.json
, which integrates with node's native ESM support and gives warnings for missing.js
suffixes.tsconfig.esm.json
ts-mocha
against the ts source code)... idea from this blogThis (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:
.ts
extensions for internal imports/exports #4106require
calls inesm
build of@opentelemetry/resources
#4642The text was updated successfully, but these errors were encountered: