Fork navigateReducer into PPR and non-PPR versions #59538

acdlite · 2023-12-12T17:11:51Z

The PPR implementation of navigateReducer is expected to diverge significantly from the existing, non-PPR implementation. So this forks them into two separate functions. This will be easier to maintain than two different implementations inside the same function, especially considering we don't expect any more changes to the non-PPR implementation.

This also reduces the chances we'll introduce an accidental regression into the non-PPR version, which is the stable one that all users (except for the ones dogfooding PPR) are currently using.

For now, the two implementations are identical. I'll start making changes in subsequent PRs.

Only one implementation will be included in the final build; the other one will be dead code eliminated because the feature check is statically inlined at build time:

export const navigateReducer = process.env.__NEXT_PPR
  ? navigateReducer_PPR
  : navigateReducer_noPPR

Closes NEXT-1856

ijjk · 2023-12-12T17:19:00Z

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
buildDuration	12.8s	12.7s	N/A
buildDurationCached	7.1s	7.3s	⚠️ +221ms
nodeModulesSize	200 MB	200 MB	⚠️ +41.9 kB
nextStartRea..uration (ms)	444ms	454ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
170-HASH.js gzip	26.7 kB	26.8 kB	N/A
199.HASH.js gzip	181 B	185 B	N/A
3f784ff6-HASH.js gzip	53.3 kB	53.3 kB	✓
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	240 B	241 B	N/A
main-HASH.js gzip	31.7 kB	31.6 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	N/A
Overall change	98.5 kB	98.5 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
_app-HASH.js gzip	195 B	194 B	N/A
_error-HASH.js gzip	183 B	182 B	N/A
amp-HASH.js gzip	501 B	501 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	255 B	255 B	✓
head-HASH.js gzip	349 B	350 B	N/A
hooks-HASH.js gzip	368 B	369 B	N/A
image-HASH.js gzip	4.27 kB	4.27 kB	N/A
index-HASH.js gzip	255 B	256 B	N/A
link-HASH.js gzip	2.61 kB	2.6 kB	N/A
routerDirect..HASH.js gzip	311 B	309 B	N/A
script-HASH.js gzip	384 B	384 B	✓
withRouter-HASH.js gzip	307 B	306 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	1.57 kB	1.57 kB	✓

Client Build Manifests

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
_buildManifest.js gzip	484 B	482 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
index.html gzip	530 B	527 B	N/A
link.html gzip	542 B	540 B	N/A
withRouter.html gzip	524 B	524 B	✓
Overall change	524 B	524 B	✓

Edge SSR bundle Size

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
edge-ssr.js gzip	93.7 kB	93.7 kB	N/A
page.js gzip	146 kB	146 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
middleware-b..fest.js gzip	627 B	622 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	37.4 kB	37.4 kB	N/A
edge-runtime..pack.js gzip	1.92 kB	1.92 kB	✓
Overall change	2.07 kB	2.07 kB	✓

Next Runtimes

	vercel/next.js canary	acdlite/next.js fork-navigatereducer-ppr	Change
app-page-exp...dev.js gzip	168 kB	168 kB	✓
app-page-exp..prod.js gzip	93.9 kB	93.9 kB	✓
app-page-tur..prod.js gzip	94.7 kB	94.7 kB	✓
app-page-tur..prod.js gzip	89.2 kB	89.2 kB	✓
app-page.run...dev.js gzip	138 kB	138 kB	✓
app-page.run..prod.js gzip	88.5 kB	88.5 kB	✓
app-route-ex...dev.js gzip	23.9 kB	23.9 kB	✓
app-route-ex..prod.js gzip	16.6 kB	16.6 kB	✓
app-route-tu..prod.js gzip	16.6 kB	16.6 kB	✓
app-route-tu..prod.js gzip	16.2 kB	16.2 kB	✓
app-route.ru...dev.js gzip	23.4 kB	23.4 kB	✓
app-route.ru..prod.js gzip	16.2 kB	16.2 kB	✓
pages-api-tu..prod.js gzip	9.37 kB	9.37 kB	✓
pages-api.ru...dev.js gzip	9.64 kB	9.64 kB	✓
pages-api.ru..prod.js gzip	9.37 kB	9.37 kB	✓
pages-turbo...prod.js gzip	21.9 kB	21.9 kB	✓
pages.runtim...dev.js gzip	22.5 kB	22.5 kB	✓
pages.runtim..prod.js gzip	21.9 kB	21.9 kB	✓
server.runti..prod.js gzip	49.4 kB	49.4 kB	✓
Overall change	929 kB	929 kB	✓

Diff details

Diff for page.js

Diff too large to display

Diff for 170-HASH.js

Diff too large to display

Commit: 1e244a1

The PPR implementation of navigateReducer is expected to diverge significantly from the existing, non-PPR implementation. So this forks them into two separate functions. This will be easier to maintain than two different implementations inside the same function, especially considering we don't expect any more changes to the non- PPR implementation. This also reduces the chances we'll introduce an accidental regression into the non-PPR version, which is the stable one that all users (except for the ones dogfooding PPR) are currently using. For now, the two implementations are identical. I'll start making changes in subsequent PRs. Only one implementation will be included in the final build; the other one will be dead code eliminated because the feature check is statically inlined at build time: ```js export const navigateReducer = process.env.__NEXT_PPR ? navigateReducer_PPR : navigateReducer_noPPR ```

ztanner · 2023-12-13T00:19:47Z

packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts

+  prefetchQueue.bump(data!)
+
+  return data!.then(
+    ([flightData, canonicalUrlOverride, postponed]) => {


It should be safe to remove the postponed handling in the non-PPR case, but I don't mind it being a separate PR if that's the plan 👍

yeah I left it in the spirit of "do nothing except copy paste the entire implementation"

ztanner · 2023-12-13T00:29:29Z

packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts

+// These implementations are expected to diverge significantly, so I've forked
+// the entire function. The one that's disabled should be dead code eliminated
+// because the check here is statically inlined at build time.
+export const navigateReducer = process.env.__NEXT_PPR


wonder if it'd be useful to annotate the action type that we dispatch to devtools with something like navigate (ppr) just to sanity check which reducer is being used on the client. But I imagine it'll be pretty obvious which one is being used without that once more functionality is wired up, so maybe not worth the effort

yeah you can just log in the reducer, or log process.env.__NEXT_PPR, since it's a static condition

Follow-up to vercel#59552, I think this one was missed because it landed around the same time as vercel#59538.

Follow-up to #59552, I think this one was missed because it landed around the same time as #59538. Closes NEXT-1869

ijjk added created-by: Next.js team type: next labels Dec 12, 2023

acdlite force-pushed the fork-navigatereducer-ppr branch 2 times, most recently from 44a71a7 to 0eabb3e Compare December 12, 2023 17:13

acdlite force-pushed the fork-navigatereducer-ppr branch from 0eabb3e to 87f1be2 Compare December 12, 2023 18:28

acdlite force-pushed the fork-navigatereducer-ppr branch from 87f1be2 to 1e244a1 Compare December 12, 2023 18:29

acdlite marked this pull request as ready for review December 12, 2023 18:29

acdlite requested review from timneutkens, ijjk, shuding, huozhi, feedthejim, ztanner and wyattjoh as code owners December 12, 2023 18:29

ztanner approved these changes Dec 13, 2023

View reviewed changes

ztanner reviewed Dec 13, 2023

View reviewed changes

acdlite merged commit 9dfeced into vercel:canary Dec 13, 2023

acdlite added a commit to acdlite/next.js that referenced this pull request Dec 13, 2023

Fix another magic segment string constant

92e33dd

Follow-up to vercel#59552, I think this one was missed because it landed around the same time as vercel#59538.

acdlite mentioned this pull request Dec 13, 2023

Fix another magic segment string constant #59591

Merged

acdlite added a commit that referenced this pull request Dec 13, 2023

Fix another magic segment string constant (#59591)

79e82d4

Follow-up to #59552, I think this one was missed because it landed around the same time as #59538. Closes NEXT-1869

github-actions bot added the locked label Dec 28, 2023

github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fork navigateReducer into PPR and non-PPR versions #59538

Fork navigateReducer into PPR and non-PPR versions #59538

acdlite commented Dec 12, 2023 •

edited

Loading

ijjk commented Dec 12, 2023 •

edited

Loading

ztanner Dec 13, 2023

acdlite Dec 13, 2023

ztanner Dec 13, 2023

acdlite Dec 13, 2023

Fork navigateReducer into PPR and non-PPR versions #59538

Fork navigateReducer into PPR and non-PPR versions #59538

Conversation

acdlite commented Dec 12, 2023 • edited Loading

ijjk commented Dec 12, 2023 • edited Loading

Stats from current PR

ztanner Dec 13, 2023

Choose a reason for hiding this comment

acdlite Dec 13, 2023

Choose a reason for hiding this comment

ztanner Dec 13, 2023

Choose a reason for hiding this comment

acdlite Dec 13, 2023

Choose a reason for hiding this comment

acdlite commented Dec 12, 2023 •

edited

Loading

ijjk commented Dec 12, 2023 •

edited

Loading