Create react server condition alias for next/navigation api #62456

huozhi · 2024-02-23T18:14:53Z

What

Introduce a react-server export condition of next/navigation, which only take effects in RSC layer. And it will only contain notFound and redirect related APIs, which can be shared in both server components and client components environment. This export excludes those APIs working with React context which are only working in client components.

Why

We fixed an issue bad alias for react-server condition of react itself in https://github.com/vercel/next.js/pull/61522/files#diff-ecb951c8d26893f6d1e4425a873b399d52346ef63eb90fba79d980cef2fabe8cL35 , this was a good fix. But we found that if you're using edge runtime with next/navigation it will error with bundling that you're attempted to import some client component hooks such as useContext from react.

So we introduced a react-server version of next/navigation that doesn't interoplate with any client hooks, can we'll bundle that one instead of original next/navigation when you're using it in server components or app routes.

Closes NEXT-2583
Closes NEXT-2519
Fixes #62187

ijjk · 2024-02-23T18:18:12Z

Failing test suites

Commit: 3ec5ef2

pnpm test-start test/e2e/app-dir/navigation/navigation.test.ts (PPR)

app dir - navigation > navigation between pages and app > should not continously initiate a mpa navigation to the same URL when router state changes

Expand output

● app dir - navigation › navigation between pages and app › should not continously initiate a mpa navigation to the same URL when router state changes

page.waitForSelector: Timeout 60000ms exceeded.
Call log:
  - waiting for locator('#link-to-slow-page')
  -   locator resolved to visible <a href="/slow-page" id="link-to-slow-page">To /slow-page</a>

  421 |     return this.chain(() => {
  422 |       return page
> 423 |         .waitForSelector(selector, { timeout, state: 'attached' })
      |          ^
  424 |         .then(async (el) => {
  425 |           // it seems selenium waits longer and tests rely on this behavior
  426 |           // so we wait for the load event fire before returning

  at waitForSelector (lib/browsers/playwright.ts:423:10)

Read more about building and testing Next.js in contributing.md.

ijjk · 2024-02-23T18:21:14Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js add-rsc-version-navigation	Change
buildDuration	13.8s	13.9s	⚠️ +153ms
buildDurationCached	7.9s	6.4s	N/A
nodeModulesSize	197 MB	197 MB	⚠️ +41.1 kB
nextStartRea..uration (ms)	408ms	410ms	N/A

Client Bundles (main, webpack) Overall increase ⚠️

	vercel/next.js canary	vercel/next.js add-rsc-version-navigation	Change
1068-HASH.js gzip	30.3 kB	30.5 kB	⚠️ +152 B
3f784ff6-HASH.js gzip	53.7 kB	53.7 kB	N/A
4944-HASH.js gzip	5.04 kB	5.04 kB	N/A
8423.HASH.js gzip	181 B	181 B	✓
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	241 B	241 B	✓
main-HASH.js gzip	32.2 kB	32.2 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	N/A
Overall change	76 kB	76.1 kB	⚠️ +152 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js add-rsc-version-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 add-rsc-version-navigation	Change
_app-HASH.js gzip	196 B	197 B	N/A
_error-HASH.js gzip	184 B	184 B	✓
amp-HASH.js gzip	503 B	505 B	N/A
css-HASH.js gzip	323 B	325 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.5 kB	✓
edge-ssr-HASH.js gzip	258 B	258 B	✓
head-HASH.js gzip	353 B	352 B	N/A
hooks-HASH.js gzip	370 B	371 B	N/A
image-HASH.js gzip	4.21 kB	4.2 kB	N/A
index-HASH.js gzip	259 B	259 B	✓
link-HASH.js gzip	2.68 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	313 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	4.01 kB	4.01 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js add-rsc-version-navigation	Change
_buildManifest.js gzip	485 B	485 B	✓
Overall change	485 B	485 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js add-rsc-version-navigation	Change
index.html gzip	529 B	526 B	N/A
link.html gzip	541 B	538 B	N/A
withRouter.html gzip	525 B	521 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js add-rsc-version-navigation	Change
edge-ssr.js gzip	94.9 kB	94.9 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 add-rsc-version-navigation	Change
middleware-b..fest.js gzip	625 B	623 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	25.4 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 add-rsc-version-navigation	Change
app-page-exp...dev.js gzip	170 kB	170 kB	N/A
app-page-exp..prod.js gzip	96.5 kB	96.6 kB	N/A
app-page-tur..prod.js gzip	98.3 kB	98.3 kB	N/A
app-page-tur..prod.js gzip	92.8 kB	92.8 kB	N/A
app-page.run...dev.js gzip	151 kB	151 kB	N/A
app-page.run..prod.js gzip	91.3 kB	91.3 kB	N/A
app-route-ex...dev.js gzip	22 kB	22 kB	N/A
app-route-ex..prod.js gzip	15 kB	15 kB	N/A
app-route-tu..prod.js gzip	15 kB	15 kB	N/A
app-route-tu..prod.js gzip	14.7 kB	14.8 kB	N/A
app-route.ru...dev.js gzip	21.7 kB	21.7 kB	N/A
app-route.ru..prod.js gzip	14.7 kB	14.8 kB	N/A
pages-api-tu..prod.js gzip	9.5 kB	9.51 kB	N/A
pages-api.ru...dev.js gzip	9.78 kB	9.79 kB	N/A
pages-api.ru..prod.js gzip	9.5 kB	9.51 kB	N/A
pages-turbo...prod.js gzip	22.3 kB	22.3 kB	N/A
pages.runtim...dev.js gzip	23 kB	23 kB	N/A
pages.runtim..prod.js gzip	22.3 kB	22.3 kB	N/A
server.runti..prod.js gzip	50.5 kB	50.5 kB	N/A
Overall change	0 B	0 B	✓

build cache Overall increase ⚠️

	vercel/next.js canary	vercel/next.js add-rsc-version-navigation	Change
0.pack gzip	1.56 MB	1.56 MB	⚠️ +2.04 kB
index.pack gzip	105 kB	104 kB	N/A
Overall change	1.56 MB	1.56 MB	⚠️ +2.04 kB

Diff details

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 1068-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-api-tu..time.prod.js

Diff too large to display

Diff for pages-api.runtime.dev.js

Diff too large to display

Diff for pages-api.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: c041a75

kwonoj · 2024-02-23T21:48:12Z

packages/next/src/build/create-compiler-aliases.ts

    head: 'next/dist/client/components/noop-head',
    dynamic: 'next/dist/api/app-dynamic',
  }

+  if (isServerOnlyLayer) {
+    mapping['navigation'] = 'next/dist/api/navigation.react-server'


should we have same for the turbopack? or it just passes new test automatically?

@kwonoj good call, we can add for turbopack too. It seems like doesn't effect turbopack yet

Tested it that turbopack doesn't have this issue, I'll create a separate PR to follow up the alias for turbopack

gnoff

One of the nice things we can do with a fork like this is just not export the invalid methods from the react server module. Instead of waiting until runtime to detect an invalid usage we can know at compile time. This also means the error won’t creep up only in some dynamic render or in a revalidate rather than during a build.

it’s unfortunate that the error message itself would be more generic (really it’s exactly the error that instigated this fix in the first place for useContext exporting from react). However I thought we already have some error specialization where we take a generic import error and replace it with something that provides better explanation.

Have you looked at that approach already?

huozhi · 2024-02-24T00:59:49Z

@gnoff I was actually using that before the last few commits, but the error message is not ideal. We also have that in SWR. You might see sth like (0, __next_navigation_mod).usePathname is not a function, which is pretty hard to determine the format and source import module. So I still keep the wrong time error there. This is a quite general issue that we cannot mock the exports for specific export condition.

The better approach is we do a follow up change that replaces the runtime checking with SWC imports checking in build time. This only resolves the bundling error to unblock the build, later we introduce the better build time checking so it can error earlier

gnoff · 2024-02-24T03:58:10Z

Makes sense!

Create react server condition alias for next/navigation api

08630b8

ijjk added area: tests created-by: Next.js team PRs by the Next.js team type: next labels Feb 23, 2024

huozhi added 2 commits February 23, 2024 19:22

test: fix missing route

f083709

apply for both dev and prod

6033a7a

huozhi mentioned this pull request Feb 23, 2024

[regression] "'useContext' is not exported from 'react'" error message in latest canary, works fine in v14.1.0 #62187

Closed

fix erroring on invalid api

fad058e

huozhi marked this pull request as ready for review February 23, 2024 20:32

huozhi requested review from timneutkens, ijjk, shuding, ztanner, feedthejim, a team and wyattjoh as code owners February 23, 2024 20:32

kwonoj reviewed Feb 23, 2024

View reviewed changes

gnoff reviewed Feb 23, 2024

View reviewed changes

share code

3ec5ef2

Merge branch 'canary' into add-rsc-version-navigation

c041a75

huozhi mentioned this pull request Feb 24, 2024

[turbopack] update edge alias #62461

Merged

gnoff approved these changes Feb 24, 2024

View reviewed changes

shuding approved these changes Feb 24, 2024

View reviewed changes

huozhi merged commit c221fc4 into canary Feb 26, 2024
66 of 69 checks passed

huozhi deleted the add-rsc-version-navigation branch February 26, 2024 12:35

huozhi mentioned this pull request Mar 11, 2024

Use SWC to valid client next/navigation hooks usage in server components #63160

Merged

github-actions bot added the locked label Mar 12, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create react server condition alias for next/navigation api #62456

Create react server condition alias for next/navigation api #62456

huozhi commented Feb 23, 2024 •

edited

ijjk commented Feb 23, 2024 •

edited

ijjk commented Feb 23, 2024 •

edited

kwonoj Feb 23, 2024

huozhi Feb 23, 2024 •

edited

huozhi Feb 26, 2024

gnoff left a comment

huozhi commented Feb 24, 2024

gnoff commented Feb 24, 2024

Create react server condition alias for next/navigation api #62456

Create react server condition alias for next/navigation api #62456

Conversation

huozhi commented Feb 23, 2024 • edited

What

Why

ijjk commented Feb 23, 2024 • edited

Failing test suites

ijjk commented Feb 23, 2024 • edited

Stats from current PR

kwonoj Feb 23, 2024

Choose a reason for hiding this comment

huozhi Feb 23, 2024 • edited

Choose a reason for hiding this comment

huozhi Feb 26, 2024

Choose a reason for hiding this comment

gnoff left a comment

Choose a reason for hiding this comment

huozhi commented Feb 24, 2024

gnoff commented Feb 24, 2024

huozhi commented Feb 23, 2024 •

edited

ijjk commented Feb 23, 2024 •

edited

ijjk commented Feb 23, 2024 •

edited

huozhi Feb 23, 2024 •

edited