-
Notifications
You must be signed in to change notification settings - Fork 26.7k
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
Use SWC to valid client next/navigation hooks usage in server components #63160
Conversation
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
buildDuration | 13.8s | 13.8s | N/A |
buildDurationCached | 7.5s | 6.2s | N/A |
nodeModulesSize | 198 MB | 198 MB | N/A |
nextStartRea..uration (ms) | 437ms | 433ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
2453-HASH.js gzip | 30.5 kB | 30.3 kB | N/A |
3304.HASH.js gzip | 181 B | 181 B | ✓ |
3f784ff6-HASH.js gzip | 53.7 kB | 53.7 kB | N/A |
8299-HASH.js gzip | 5.04 kB | 5.04 kB | N/A |
framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
main-app-HASH.js gzip | 241 B | 242 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 | 45.4 kB | 45.4 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | 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.21 kB | 4.21 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.57 kB | 6.57 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
_buildManifest.js gzip | 481 B | 484 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
index.html gzip | 528 B | 530 B | N/A |
link.html gzip | 541 B | 542 B | N/A |
withRouter.html gzip | 524 B | 524 B | ✓ |
Overall change | 524 B | 524 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
edge-ssr.js gzip | 95.1 kB | 95.1 kB | N/A |
page.js gzip | 3.07 kB | 3.07 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 627 B | 624 B | N/A |
middleware-r..fest.js gzip | 151 B | 151 B | ✓ |
middleware.js gzip | 25.5 kB | 25.5 kB | N/A |
edge-runtime..pack.js gzip | 839 B | 839 B | ✓ |
Overall change | 990 B | 990 B | ✓ |
Next Runtimes
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 171 kB | 171 kB | ✓ |
app-page-exp..prod.js gzip | 96.6 kB | 96.6 kB | ✓ |
app-page-tur..prod.js gzip | 98.3 kB | 98.3 kB | ✓ |
app-page-tur..prod.js gzip | 92.8 kB | 92.8 kB | ✓ |
app-page.run...dev.js gzip | 149 kB | 149 kB | ✓ |
app-page.run..prod.js gzip | 91.3 kB | 91.3 kB | ✓ |
app-route-ex...dev.js gzip | 21.3 kB | 21.3 kB | ✓ |
app-route-ex..prod.js gzip | 15 kB | 15 kB | ✓ |
app-route-tu..prod.js gzip | 15 kB | 15 kB | ✓ |
app-route-tu..prod.js gzip | 14.8 kB | 14.8 kB | ✓ |
app-route.ru...dev.js gzip | 21 kB | 21 kB | ✓ |
app-route.ru..prod.js gzip | 14.8 kB | 14.8 kB | ✓ |
pages-api-tu..prod.js gzip | 9.52 kB | 9.52 kB | ✓ |
pages-api.ru...dev.js gzip | 9.8 kB | 9.8 kB | ✓ |
pages-api.ru..prod.js gzip | 9.52 kB | 9.52 kB | ✓ |
pages-turbo...prod.js gzip | 22.3 kB | 22.3 kB | ✓ |
pages.runtim...dev.js gzip | 22.9 kB | 22.9 kB | ✓ |
pages.runtim..prod.js gzip | 22.3 kB | 22.3 kB | ✓ |
server.runti..prod.js gzip | 50.5 kB | 50.5 kB | ✓ |
Overall change | 948 kB | 948 kB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | vercel/next.js swc/rsc-check-next-navigation | Change | |
---|---|---|---|
0.pack gzip | 1.56 MB | 1.56 MB | N/A |
index.pack gzip | 105 kB | 105 kB | |
Overall change | 105 kB | 105 kB |
Diff details
Diff for middleware.js
Diff too large to display
Diff for 2453-HASH.js
Diff too large to display
Tests Passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to see this land. I do think we can make the error message more helpful and left a suggestion in case you want to tackle that here and now vs in a follow up PR
// So we need to check for the first part of the message. | ||
const normalizedSource = await session.getRedboxSource() | ||
expect(normalizedSource).toContain( | ||
`You're importing a component that needs ${hook}. It only works in a Client Component but none of its parents are marked with "use client"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't change this string so it's probably a long standing message but I wonder if we should change this. First, you may not be importing a component at all. You may be importing some random value from a file which just happens to be importing this hook. Maybe something like this would be more true to what is happening
${pathToFileContainingClientOnlyImport} is being imported in a Server Component context by ${pathToFileDoingTheImporting} but it imports a client only ${ClientOnlyImport} from ${package specifier}. If you intended to only use this file in a Client Component context ensure that it or at least one of it's parent modules is marked with "use client". If you want to use one or more exports in a Server Component context extract them into their own module that does not depend on any client only imports from ${package specifier}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to improve that as well, we have the similar error message like Maybe one of these should be marked as a client entry 'use client'
and printing the module trace, mainly for React client hooks before. I can improve this in a separate PR 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also remove clientHookInServerComponentError
from the client implementation of navigation hooks?
Good catch, deleted it in 183f449 |
What
Use SWC to check invalid client hooks of
next/navigation
imports in server components.Follow up of #62456
Remove the runtime error APIs for
next/navigation
rsc version.Add
next/navigation
react-server version alias in turbopack.This PR also refactored the invalid server layer APIs detection into a map, where key is import path and value is an array of client APIs. During the traversing we will get the import source easily, this makes extending the logic much easier
Why
Previously we're using the runtime error to check it, but it has to run first then the error will be thrown. If we error first in build time with this check it's much faster and we this align on both side between webpack and turbopack.