Skip to content
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

feat(node): Make ESM output valid Node ESM #10928

Merged
merged 15 commits into from
Mar 8, 2024
Merged

feat(node): Make ESM output valid Node ESM #10928

merged 15 commits into from
Mar 8, 2024

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Mar 5, 2024

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 feat: Emit *.mjs files across all packages #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

@timfish timfish changed the title feat(esm): ESM Take 10 feat(node): Make ESM output valid Node ESM Mar 5, 2024
@AbhiPrasad
Copy link
Member

Tim you're a hero 🫡

@mydea
Copy link
Member

mydea commented Mar 7, 2024

This may work together with: #10960

AbhiPrasad added a commit that referenced this pull request Mar 7, 2024
we would like to start using node protocol imports in the SDK to unblock
#10960 and
#10928

This requires us to have a minimum supported Node version of `14.18.0`
as per https://2ality.com/2021/12/node-protocol-imports.html
@timfish
Copy link
Collaborator Author

timfish commented Mar 7, 2024

I make a load of changes to the deprecated ./packages/node package to get the tests passing without updating all the SDKs to the new Node package. To do the http > node:http changes I had to update the node types and then fix/ignore all the issue that introduced...

@AbhiPrasad
Copy link
Member

We can rm -rf size-check with codecov bundle analysis https://docs.codecov.com/docs/javascript-bundle-analysis if it refuses to work with esm.

@timfish timfish marked this pull request as ready for review March 8, 2024 17:50
@timfish timfish requested a review from AbhiPrasad March 8, 2024 17:51
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

@AbhiPrasad AbhiPrasad merged commit 7f2e804 into develop Mar 8, 2024
111 checks passed
@AbhiPrasad AbhiPrasad deleted the esm/again branch March 8, 2024 17:53
@AbhiPrasad
Copy link
Member

Opened prisma/prisma#23410 for prisma being CJS only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants