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

feat(error-overlay): version staleness in Pages Router #62942

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Mar 6, 2024

What?

Before:
image

After:
image

Why?

The same way we indicate version staleness to the user in the App Router, we should do the same in the Pages Router for a better DX.

How?

The server already sent this info to the Pages error overlay, but we did not handle it correctly previously. During a sync event with the server, we now emit the version info and properly pass it to the error overlay components.

Note: Currently the Pages/App overlays duplicate a lot of code. To prevent out-of-sync behavior in the future like this, I started another PR #62939 that will unify the repeated parts.

Slack thread
Closes NEXT-2711

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Mar 6, 2024
@ijjk
Copy link
Member

ijjk commented Mar 6, 2024

Failing test suites

Commit: 547d950

pnpm test-dev test/e2e/app-dir/params-hooks-compat/index.test.ts

  • app-dir - params hooks compat > should only access search params with useSearchParams
Expand output

● app-dir - params hooks compat › should only access search params with useSearchParams

expect(received).toEqual(expected) // deep equality

- Expected  - 3
+ Received  + 1

- Object {
-   "q": "pages",
- }
+ Object {}

  18 |
  19 |       expect(appSearchparamsJSON).toEqual({ q: 'app' })
> 20 |       expect(pagesSearchparamsJSON).toEqual({ q: 'pages' })
     |                                     ^
  21 |     })
  22 |
  23 |     it('should only access path params with useParams', async () => {

  at Object.toEqual (e2e/app-dir/params-hooks-compat/index.test.ts:20:37)

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

pnpm test-dev test/e2e/app-dir/rsc-basic/rsc-basic.test.ts

  • app dir - rsc basics > should be able to navigate between rsc routes
Expand output

● app dir - rsc basics › should be able to navigate between rsc routes

request.allHeaders: Target page, context or browser has been closed

  145 |           page.on('request', (request) => {
  146 |             requestsCount++
> 147 |             return request.allHeaders().then((headers) => {
      |                            ^
  148 |               if (
  149 |                 headers['RSC'.toLowerCase()] === '1' &&
  150 |                 // Prefetches also include `RSC`

  at Page.allHeaders (e2e/app-dir/rsc-basic/rsc-basic.test.ts:147:28)

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

@ijjk
Copy link
Member

ijjk commented Mar 6, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
buildDuration 13.8s 13.9s N/A
buildDurationCached 7.5s 6.2s N/A
nodeModulesSize 198 MB 198 MB ⚠️ +6.35 kB
nextStartRea..uration (ms) 439ms 413ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
2453-HASH.js gzip 30.5 kB 30.5 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 242 B 242 B
main-HASH.js gzip 32.1 kB 32.2 kB N/A
webpack-HASH.js gzip 1.68 kB 1.68 kB N/A
Overall change 45.6 kB 45.6 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js feat/version-staleness-pages 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.2 kB 4.2 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.56 kB 6.56 kB
Client Build Manifests
vercel/next.js canary vercel/next.js feat/version-staleness-pages 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 feat/version-staleness-pages Change
index.html gzip 528 B 529 B N/A
link.html gzip 542 B 541 B N/A
withRouter.html gzip 524 B 523 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
edge-ssr.js gzip 95 kB 95 kB N/A
page.js gzip 3.06 kB 3.07 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
middleware-b..fest.js gzip 623 B 623 B
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 1.61 kB 1.61 kB
Next Runtimes
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
app-page-exp...dev.js gzip 171 kB 171 kB
app-page-exp..prod.js gzip 96.7 kB 96.7 kB
app-page-tur..prod.js gzip 98.5 kB 98.5 kB
app-page-tur..prod.js gzip 92.9 kB 92.9 kB
app-page.run...dev.js gzip 149 kB 149 kB
app-page.run..prod.js gzip 91.4 kB 91.4 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.54 kB 9.54 kB
pages-api.ru...dev.js gzip 9.81 kB 9.81 kB
pages-api.ru..prod.js gzip 9.54 kB 9.54 kB
pages-turbo...prod.js gzip 22.3 kB 22.3 kB
pages.runtim...dev.js gzip 23 kB 23 kB
pages.runtim..prod.js gzip 22.3 kB 22.3 kB
server.runti..prod.js gzip 50.6 kB 50.6 kB
Overall change 949 kB 949 kB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js feat/version-staleness-pages Change
0.pack gzip 1.56 MB 1.56 MB ⚠️ +1.11 kB
index.pack gzip 105 kB 106 kB ⚠️ +404 B
Overall change 1.66 MB 1.66 MB ⚠️ +1.51 kB
Diff details
Diff for middleware.js

Diff too large to display

Commit: 547d950

@balazsorban44 balazsorban44 marked this pull request as ready for review March 6, 2024 14:53
@balazsorban44 balazsorban44 requested review from manovotny and StephDietz and removed request for a team March 6, 2024 14:53
@balazsorban44 balazsorban44 enabled auto-merge (squash) March 6, 2024 15:51
@balazsorban44 balazsorban44 merged commit 0a73e89 into canary Mar 6, 2024
68 checks passed
@balazsorban44 balazsorban44 deleted the feat/version-staleness-pages branch March 6, 2024 16:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
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.

3 participants