feat: Emit *.mjs files across all packages#10833
feat: Emit *.mjs files across all packages#10833AbhiPrasad wants to merge 21 commits intodevelopfrom
Conversation
b0d0d93 to
921c283
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
Node/Remix Integration tests are now passing. Still a few more issues to work through... |
|
|
@timfish We likely have to update the paths here: |
This comment was marked as outdated.
This comment was marked as outdated.
|
The nextjs e2e test failures are weird. Next.js should not see the "wrapping target file" paths at all. They should all be bundled away with the rollup call here: |
IIRC my project's |
|
I've fixed this previous issue with nextjs which was caused by the build wrapping script being removed. This resulted in cjs templates being used rather than esm. The server tests are all now passing! However, most of the client playwright tests are failing: 
|
|
The |
|
Closing in favour of #10928 |
Some bundlers, especially older ones, don't support resolving `.mjs`
files out of the box. To retain compatibility with these bundlers
without requiring users to adjust their configurations, we need to keep
all our ESM output as `.js` files.
So node.js loads these as modules, we output a `package.json` into each
`./build/esm` output directory containing simply `{ "type": "module" }`.
This works because whenever node.js is asked to load a `.js` file, it
tries to work out whether this is CJS or ESM by searching up through
parent directories until it finds a `package.json` with the `type` set.
This PR:
- Adds a Rollup plugin that injects the `package.json` into the ESM
output
- Adds the package `exports` that @AbhiPrasad worked out for #10833
- Fixes an import issue with `next/router` which is CJS (at least in
v10)
- Fixes an import issue with `@prisma/instrumentation` which is CJS
- Ensures that CJS only integrations are not included in the
`@sentry/node` default integrations when running as ESM
This PR also makes some unrelated changes:
- Changes to the old Node SDKs to allow the tests to pass
- Removes the bundle size analysis CI job as it doesn't appears to be
compatible with the node ESM output
#10046
On the road of proper esm support, this first change introduces
package.jsonexports to each of our packages. Now lets see what breaks 😭This PR has some breaking changes or things that have been disabled to get the build working and tests passing which will need to be revisited later:
modulesIntegrationusesrequireto get the module list and makes no sense for ESMrequire('inspector')or `dynamicRequire('inspector') have been replaced with regular imports. We can't use async import because it introduces async where it can't be supported. This change means we will break Vercel/pkg apps since they build Node without inspector. Vercel/pkg has been deprecated as of Jan 2024.nativeNodeFetchIntegrationis no longer in the default integrations and the export has been removed for now. This integration usesrequireand when I triedimport('opentelemetry-instrumentation-fetch-node')it seemed seemed to break Rollup output in a similar way to this: preserveModules creates wrong output rollup/rollup#3743Other outstanding issues:
Error: Failed to resolve entry for package "https"