Skip to content

ref(nextjs): Replace multiplexer with conditional exports #11442

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

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Apr 5, 2024

We can replace the SDK multiplexer with conditional exports. I found this while preparing the Next.js SDK for using @vercel/otel.

This is preferrable over the current solution because the loaders will not work any longer with turbopack.

@lforst lforst changed the title feat(nextjs): Prepare Next.js usage for using @vercel/otel feat(nextjs): Replace multiplexer with conditional exports Apr 5, 2024
@lforst lforst changed the title feat(nextjs): Replace multiplexer with conditional exports ref(nextjs): Replace multiplexer with conditional exports Apr 5, 2024
@lforst lforst requested a review from AbhiPrasad April 5, 2024 12:41
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.

Wow end of an era

"default": "./build/esm/edge/index.js"
},
"workerd": {
"default": "./build/esm/edge/index.js"
Copy link
Member

Choose a reason for hiding this comment

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

These are always esm only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, in general I think nextjs can cope with only having esm but I'm gonna try to add cjs exports too.

@lforst lforst merged commit a02e6ff into develop Apr 8, 2024
@lforst lforst deleted the lforst-edge-otel-preparation branch April 8, 2024 08:40
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this pull request Apr 19, 2024
"browser": {
"import": "./build/esm/index.client.js",
"require": "./build/cjs/index.client.js"
},
"node": "./build/cjs/index.server.js",
"import": "./build/esm/index.server.js",

Choose a reason for hiding this comment

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

Hi @lforst
I noticed in this PR that node exports take precedence over import. Is this expected? This causes the resolution to always fall back to the CommonJS version in a Node environment, regardless of whether ESM is used.

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