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

Fix nested esm package default import resolving mismatch #57784

Merged
merged 3 commits into from
Oct 31, 2023

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Oct 30, 2023

For app router bundling layers "SSR rendering" and "browser" layer, which are used for server side rendering and client, we should still apply the module resolving rules to all assets since we bundled everything, removed the default exclude conditions as they'll not apply the rules to node_modules. That could cause the mismatch resolving for package.

E.g. You have two dual packages A and B are both compatible for ESM and CJS, and both have default export. B is depent on A, but when you import B in a client component that will be SSR'd, it's picking up the CJS asset like the case described in #57584 .

Fixes #57584
Closes NEXT-1702

@ijjk
Copy link
Member

ijjk commented Oct 30, 2023

Tests Passed

@ijjk
Copy link
Member

ijjk commented Oct 30, 2023

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
buildDuration 10.1s 10.3s ⚠️ +188ms
buildDurationCached 5.9s 5.8s N/A
nodeModulesSize 175 MB 175 MB ⚠️ +3.19 kB
nextStartRea..uration (ms) 414ms 416ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
199-HASH.js gzip 30 kB 30 kB N/A
3f784ff6-HASH.js gzip 53.2 kB 53.2 kB
99.HASH.js gzip 182 B 182 B
framework-HASH.js gzip 45.5 kB 45.5 kB
main-app-HASH.js gzip 253 B 252 B N/A
main-HASH.js gzip 33.1 kB 33.1 kB N/A
webpack-HASH.js gzip 1.75 kB 1.75 kB N/A
Overall change 98.9 kB 98.9 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
_app-HASH.js gzip 206 B 205 B N/A
_error-HASH.js gzip 182 B 181 B N/A
amp-HASH.js gzip 506 B 507 B N/A
css-HASH.js gzip 322 B 323 B N/A
dynamic-HASH.js gzip 2.59 kB 2.59 kB N/A
edge-ssr-HASH.js gzip 260 B 259 B N/A
head-HASH.js gzip 350 B 347 B N/A
hooks-HASH.js gzip 369 B 368 B N/A
image-HASH.js gzip 4.38 kB 4.38 kB N/A
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 2.67 kB 2.67 kB N/A
routerDirect..HASH.js gzip 316 B 318 B N/A
script-HASH.js gzip 385 B 383 B N/A
withRouter-HASH.js gzip 319 B 320 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 362 B 362 B
Client Build Manifests
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
_buildManifest.js gzip 484 B 484 B
Overall change 484 B 484 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
index.html gzip 528 B 527 B N/A
link.html gzip 541 B 543 B N/A
withRouter.html gzip 523 B 522 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size Overall increase ⚠️
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
edge-ssr.js gzip 96.1 kB 96.1 kB N/A
page.js gzip 140 kB 140 kB ⚠️ +119 B
Overall change 140 kB 140 kB ⚠️ +119 B
Middleware size
vercel/next.js canary vercel/next.js fix/code-split-bundle-main-field Change
middleware-b..fest.js gzip 623 B 626 B N/A
middleware-r..fest.js gzip 150 B 151 B N/A
middleware.js gzip 23 kB 23 kB
edge-runtime..pack.js gzip 1.92 kB 1.92 kB
Overall change 25 kB 25 kB
Diff details
Diff for page.js

Diff too large to display

Commit: ebbd04a

@huozhi huozhi marked this pull request as ready for review October 30, 2023 23:47
@kodiakhq kodiakhq bot merged commit cd821c8 into canary Oct 31, 2023
99 of 104 checks passed
@kodiakhq kodiakhq bot deleted the fix/code-split-bundle-main-field branch October 31, 2023 00:31
@khuezy
Copy link
Contributor

khuezy commented Oct 31, 2023

Hoping this will resolve this too: #57622
Sounds like a similar problem.
Edit: nope

@laugharn
Copy link
Contributor

laugharn commented Nov 2, 2023

I'm pretty sure this PR is unfortunately leading to some libraries breaking. I put together minimal repro with @mux/mux-player-react here: https://github.com/laugharn/next-mux-repro

@jmeistrich
Copy link

This is also breaking Legend-State: LegendApp/legend-state#222

Is there something we need to change in our bundling to be compatible with this?

@PelosiTech
Copy link

PelosiTech commented Nov 11, 2023

Breaking change as well here with @opentelemtry

⨯ ../../../node_modules/@opentelemetry/api/build/esm/context-api.js:18:0
Module not found: Can't resolve './api/context'

This is just straight from create next-app with no changes

@lukasIO
Copy link

lukasIO commented Nov 17, 2023

minimal reproductions of the issue in #57962 and #58592

@LuisGilGB
Copy link

This addition also seemed to break apps that use domino (so, by extension, apps that use turndown and, by extension of the latter, apps that use remirror).

  • Sandbox to reproduce (builds break due to with statements being used in strict mode, the sloppy.js file at domino is where logic that must avoid JavaScrip's strict mode is encapsulated).

I've been trying and yes, the first canary release with this bug is 14.0.2-canary.1. Still present at 14.0.4-canary.18.

Related stuff:

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Styled-components broken in 14.0.0
9 participants