-
Notifications
You must be signed in to change notification settings - Fork 27k
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 client boundary inheritance for barrel optimization #64467
Conversation
Failing test suitesCommit: 708ec7f
Expand output● create-next-app --app (App Router) › should create JavaScript project with --js flag
● create-next-app --app (App Router) › should create TypeScript project with --ts flag
● create-next-app --app (App Router) › should create project inside "src" directory with --src-dir flag
● create-next-app --app (App Router) › should create TailwindCSS project with --tailwind flag
Read more about building and testing Next.js in contributing.md.
Expand output● create-next-app --no-app (Pages Router) › should create JavaScript project with --js flag
● create-next-app --no-app (Pages Router) › should create TypeScript project with --ts flag
● create-next-app --no-app (Pages Router) › should create project inside "src" directory with --src-dir flag
● create-next-app --no-app (Pages Router) › should create TailwindCSS project with --tailwind flag
Read more about building and testing Next.js in contributing.md. |
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
buildDuration | 14.4s | 14.5s | |
buildDurationCached | 8.9s | 6.7s | N/A |
nodeModulesSize | 199 MB | 199 MB | |
nextStartRea..uration (ms) | 405ms | 385ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
2453-HASH.js gzip | 31.4 kB | 31.4 kB | N/A |
3304.HASH.js gzip | 181 B | 181 B | ✓ |
3f784ff6-HASH.js gzip | 53.7 kB | 53.7 kB | ✓ |
8299-HASH.js gzip | 5.1 kB | 5.1 kB | N/A |
framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
main-app-HASH.js gzip | 242 B | 240 B | N/A |
main-HASH.js gzip | 32.2 kB | 32.2 kB | N/A |
webpack-HASH.js gzip | 1.68 kB | 1.68 kB | N/A |
Overall change | 99 kB | 99 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | 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/barrel-optimization-inherit | Change | |
---|---|---|---|
_app-HASH.js gzip | 196 B | 197 B | N/A |
_error-HASH.js gzip | 184 B | 184 B | ✓ |
amp-HASH.js gzip | 505 B | 505 B | ✓ |
css-HASH.js gzip | 324 B | 325 B | N/A |
dynamic-HASH.js gzip | 2.5 kB | 2.5 kB | N/A |
edge-ssr-HASH.js gzip | 258 B | 258 B | ✓ |
head-HASH.js gzip | 352 B | 352 B | ✓ |
hooks-HASH.js gzip | 370 B | 371 B | N/A |
image-HASH.js gzip | 4.27 kB | 4.27 kB | ✓ |
index-HASH.js gzip | 259 B | 259 B | ✓ |
link-HASH.js gzip | 2.67 kB | 2.67 kB | N/A |
routerDirect..HASH.js gzip | 314 B | 312 B | N/A |
script-HASH.js gzip | 386 B | 386 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 6.63 kB | 6.63 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
_buildManifest.js gzip | 483 B | 485 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
index.html gzip | 530 B | 529 B | N/A |
link.html gzip | 541 B | 541 B | ✓ |
withRouter.html gzip | 525 B | 523 B | N/A |
Overall change | 541 B | 541 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
edge-ssr.js gzip | 95.6 kB | 95.6 kB | N/A |
page.js gzip | 3.06 kB | 3.06 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 624 B | 623 B | N/A |
middleware-r..fest.js gzip | 155 B | 156 B | N/A |
middleware.js gzip | 25.5 kB | 25.5 kB | N/A |
edge-runtime..pack.js gzip | 839 B | 839 B | ✓ |
Overall change | 839 B | 839 B | ✓ |
Next Runtimes
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 171 kB | 171 kB | ✓ |
app-page-exp..prod.js gzip | 97.5 kB | 97.5 kB | ✓ |
app-page-tur..prod.js gzip | 99.2 kB | 99.2 kB | ✓ |
app-page-tur..prod.js gzip | 93.5 kB | 93.5 kB | ✓ |
app-page.run...dev.js gzip | 145 kB | 145 kB | ✓ |
app-page.run..prod.js gzip | 92 kB | 92 kB | ✓ |
app-route-ex...dev.js gzip | 21.5 kB | 21.5 kB | ✓ |
app-route-ex..prod.js gzip | 15.2 kB | 15.2 kB | ✓ |
app-route-tu..prod.js gzip | 15.2 kB | 15.2 kB | ✓ |
app-route-tu..prod.js gzip | 14.9 kB | 14.9 kB | ✓ |
app-route.ru...dev.js gzip | 21.1 kB | 21.1 kB | ✓ |
app-route.ru..prod.js gzip | 14.9 kB | 14.9 kB | ✓ |
pages-api-tu..prod.js gzip | 9.55 kB | 9.55 kB | ✓ |
pages-api.ru...dev.js gzip | 9.82 kB | 9.82 kB | ✓ |
pages-api.ru..prod.js gzip | 9.55 kB | 9.55 kB | ✓ |
pages-turbo...prod.js gzip | 22.5 kB | 22.5 kB | ✓ |
pages.runtim...dev.js gzip | 23.1 kB | 23.1 kB | ✓ |
pages.runtim..prod.js gzip | 22.5 kB | 22.5 kB | ✓ |
server.runti..prod.js gzip | 51.3 kB | 51.3 kB | ✓ |
Overall change | 948 kB | 948 kB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | vercel/next.js fix/barrel-optimization-inherit | Change | |
---|---|---|---|
0.pack gzip | 1.59 MB | 1.58 MB | N/A |
index.pack gzip | 106 kB | 107 kB | |
Overall change | 106 kB | 107 kB |
Diff details
Diff for middleware.js
Diff too large to display
The commit on Material UI that likely surfaced this problem: mui/material-ui#40663. It was released in Material UI v5.15.7. |
Thank you @oliviertassinari to address this on mui side! I added a new test for this specific case to avoid regression in the future in 0e8fffe |
@huozhi awesome, thanks for the fix. |
…)" This reverts commit cb8fb32.
### What Inherit the client boudaries from the found matched target in load barrel ### Why The root cause with the barrel transform, we missed the client boundary directive during the transform. Since the new version of mui's case looks like this: Import path page.js (server module) -> `<module>/index.js` (shared module) -> `<module/subpath>/index.js` (client module) -> `<module/subpath/sub-module.js> (client module) After our transform, we lost the `"use client"` which causes the mismatch of the transform: In `rsc` layer: the file pointing to the sub module entry (`<module/subpath>/index.js`), but without the client boundary. Fixes #64369 Closes NEXT-3101
@oliviertassinari Do you have an ETA that will release this in a patch? |
@huozhi The PR I linked in #64467 (comment) is what I believe surfaced this bug (it's a step for Material UI toward exporting server components). The last stable release of Material UI is v5.15.15 which was 16 days ago. I see two options:
cc @DiegoAndai |
There're still a few files need to tackle such as the barrel files in mui/material, e.g. mui-material/src/Container/index.js. The issue is that when you use |
Hey! (by sheer coincidence) we opened a PR yesterday to remove all " I rephrase @oliviertassinari's question: is the next Next.js stable release scheduled to be in >4 weeks? And will that include #64681? With this info we could plan our next steps on the MUI side. |
I'm unsure about when we could do the release yet, but certain that it will include that fix. |
@huozhi I'm confused by this one. Is this a client/server bundle issue, a tree-shaking issue, or something else? I can blame the error:
with #46171 as the origin of when the error message was added.
#64681. So I guess it's why it was working before .
This seems to be OK, what's the downside of doing this? |
It's a tree-shaking issue, it will work properly once the fix on canary is out on stable release. I mean if we want to get best optimization for v5, it's good to remove the "use client" in barrel file where it contains |
I'm going to be checking the news canary versions, because I'm facing the same the MUI's issues but in my website when I build to
|
@SirFrey please submit a new issue with reproduction 🙏 |
### What * Remove the change added in #64467, but still kept tests. * Add more tests for mixed imports (component and function) from shared component with `optimizePackageImports` ### Why The fix in #64467 was not correct, as mixing `export *` with `"use client"` should error if Next.js detect it properly. When there's mixed exports, that a shared function will become a client reference if the target file inherits the client boundary. The original issue #64467 fixed was actually related to tree-shaking, which is fixed in #64681. Closes NEXT-3197
### What * Remove the change added in #64467, but still kept tests. * Add more tests for mixed imports (component and function) from shared component with `optimizePackageImports` ### Why The fix in #64467 was not correct, as mixing `export *` with `"use client"` should error if Next.js detect it properly. When there's mixed exports, that a shared function will become a client reference if the target file inherits the client boundary. The original issue #64467 fixed was actually related to tree-shaking, which is fixed in #64681. Closes NEXT-3197
@SirFrey The proper fix is published in 14.2.3, please upgrade to the new version |
@huozhi Updated and working 😄 !!! |
What
Inherit the client boudaries from the found matched target in load barrel
Why
The root cause with the barrel transform, we missed the client boundary directive during the transform.
Since the new version of mui's case looks like this:
Import path
page.js (server module) ->
<module>/index.js
(shared module) -><module/subpath>/index.js
(client module) -> `<module/subpath/sub-module.js> (client module)After our transform, we lost the
"use client"
which causes the mismatch of the transform:In
rsc
layer: the file pointing to the sub module entry (<module/subpath>/index.js
), but without the client boundary.Fixes #64369
Closes NEXT-3101