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.

Sign up for GitHub

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

Jump to bottom

backport: fix prefetch bailout detection for nested loading segments #70618

Merged
merged 1 commit into from
Sep 30, 2024
Merged

backport: fix prefetch bailout detection for nested loading segments #70618

merged 1 commit into from
Sep 30, 2024

Conversation

ztanner
Copy link
Member

@ztanner ztanner commented Sep 30, 2024 •
edited
Loading

Backports:

Fixes #70527

Note: Turbopack dev failures seem to be pre-existing on the base branch.

@ztanner
fix prefetch bailout detection for nested loading segments (#67358)
929cb69 
When PPR is off, app router prefetches will render the component tree up
until it encounters a `loading` segment, at which point it'll just
return some metadata about the segment and won't do any further
rendering. This is an optimization to ensure prefetches are lightweight
and don't potentially invoke expensive dynamic subtrees. However,
there's a bug in this logic that is causing it to bail unexpectedly if a
segment deeper in the tree contained a `loading.js` file.

This would mean the loading state wouldn't be triggered until the second
request for the full RSC data is initiated, resulting in an unintended
delta between when a link is clicked and the loading state is shown.

The `shouldSkipComponentTree` flag was incorrectly being set to `true`
even if the `loading.js` segment appeared deeper in the tree. Prefetch
requests from the client will always contain `FlightRouterState`, so the
logic to check for loading deeper in the tree will always be missed.

This removes the `flightRouterState` check as it doesn't make sense:
prefetches will currently _always_ include the `flightRouterState`,
causing this to always short-circuit. I believe that this check is
vestigial from when we were generating static `prefetch.rsc` outputs
which is no longer the case.

This mirrors a [similar
check](https://github.com/vercel/next.js/blob/b87d8fc49983a3be568517d7ae14087749bb8ce3/packages/next/src/server/app-render/create-component-tree.tsx#L393)
when determining if parallel route(s) should be rendered.
@ijjk
Copy link
Member

ijjk commented Sep 30, 2024

Failing test suites

Commit: 929cb69

pnpm test test/integration/eslint/test/next-lint.test.js

  • Next Lint > First Time Setup > installs eslint and eslint-config-next as devDependencies if missing with yarn
  • Next Lint > First Time Setup > creates .eslintrc.json file with a default configuration
  • Next Lint > First Time Setup > creates .eslintrc.json file with a default app router configuration
  • Next Lint > First Time Setup > shows a successful message when completed
Expand output

● Next Lint › First Time Setup › installs eslint and eslint-config-next as devDependencies if missing with yarn

ENOENT: no such file or directory, open '/tmp/aek3cw2vmrm/package.json'

● Next Lint › First Time Setup › creates .eslintrc.json file with a default configuration

ENOENT: no such file or directory, open '/tmp/ki0ayicgbwd/package.json'

● Next Lint › First Time Setup › creates .eslintrc.json file with a default app router configuration

ENOENT: no such file or directory, open '/tmp/lkbx5jlvslg/package.json'

● Next Lint › First Time Setup › shows a successful message when completed

ENOENT: no such file or directory, open '/tmp/eyjkojw2gsh/package.json'

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

@ijjk
Copy link
Member

ijjk commented Sep 30, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
buildDuration 16.8s 15.1s N/A
buildDurationCached 7.8s 6.8s N/A
nodeModulesSize 200 MB 200 MB ⚠️ +5.2 kB
nextStartRea..uration (ms) 406ms 407ms N/A
Client Bundles (main, webpack)
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
1103-HASH.js gzip 31.6 kB 31.6 kB N/A
1a9f679d-HASH.js gzip 53.7 kB 53.7 kB N/A
335-HASH.js gzip 5.05 kB 5.05 kB ✓
7953.HASH.js gzip 181 B 181 B ✓
framework-HASH.js gzip 44.9 kB 44.9 kB ✓
main-app-HASH.js gzip 244 B 244 B ✓
main-HASH.js gzip 32.3 kB 32.3 kB N/A
webpack-HASH.js gzip 1.68 kB 1.68 kB N/A
Overall change 50.3 kB 50.3 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB ✓
Overall change 39.4 kB 39.4 kB ✓
Client Pages
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
_app-HASH.js gzip 195 B 197 B N/A
_error-HASH.js gzip 184 B 184 B ✓
amp-HASH.js gzip 501 B 506 B N/A
css-HASH.js gzip 323 B 324 B N/A
dynamic-HASH.js gzip 1.82 kB 1.82 kB N/A
edge-ssr-HASH.js gzip 259 B 258 B N/A
head-HASH.js gzip 350 B 352 B N/A
hooks-HASH.js gzip 372 B 371 B N/A
image-HASH.js gzip 4.23 kB 4.23 kB ✓
index-HASH.js gzip 259 B 258 B N/A
link-HASH.js gzip 2.68 kB 2.68 kB N/A
routerDirect..HASH.js gzip 316 B 315 B N/A
script-HASH.js gzip 387 B 387 B ✓
withRouter-HASH.js gzip 311 B 310 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B ✓
Overall change 4.9 kB 4.9 kB ✓
Client Build Manifests
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
_buildManifest.js gzip 483 B 483 B ✓
Overall change 483 B 483 B ✓
Rendered Page Sizes
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
index.html gzip 528 B 529 B N/A
link.html gzip 542 B 541 B N/A
withRouter.html gzip 525 B 525 B ✓
Overall change 525 B 525 B ✓
Edge SSR bundle Size
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
edge-ssr.js gzip 95.4 kB 95.4 kB ✓
page.js gzip 3.06 kB 3.06 kB N/A
Overall change 95.4 kB 95.4 kB ✓
Middleware size
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
middleware-b..fest.js gzip 658 B 659 B N/A
middleware-r..fest.js gzip 156 B 156 B ✓
middleware.js gzip 25.5 kB 25.5 kB N/A
edge-runtime..pack.js gzip 839 B 839 B ✓
Overall change 995 B 995 B ✓
Next Runtimes
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
app-page-exp...dev.js gzip 171 kB 171 kB N/A
app-page-exp..prod.js gzip 98.1 kB 98.1 kB ✓
app-page-tur..prod.js gzip 99.8 kB 99.8 kB N/A
app-page-tur..prod.js gzip 94.1 kB 94.1 kB ✓
app-page.run...dev.js gzip 145 kB 145 kB N/A
app-page.run..prod.js gzip 92.6 kB 92.6 kB N/A
app-route-ex...dev.js gzip 22 kB 22 kB ✓
app-route-ex..prod.js gzip 15.5 kB 15.5 kB ✓
app-route-tu..prod.js gzip 15.5 kB 15.5 kB ✓
app-route-tu..prod.js gzip 15.2 kB 15.2 kB ✓
app-route.ru...dev.js gzip 21.7 kB 21.7 kB ✓
app-route.ru..prod.js gzip 15.2 kB 15.2 kB ✓
pages-api-tu..prod.js gzip 9.58 kB 9.58 kB ✓
pages-api.ru...dev.js gzip 9.85 kB 9.85 kB ✓
pages-api.ru..prod.js gzip 9.57 kB 9.57 kB ✓
pages-turbo...prod.js gzip 22.5 kB 22.5 kB ✓
pages.runtim...dev.js gzip 23.2 kB 23.2 kB ✓
pages.runtim..prod.js gzip 22.5 kB 22.5 kB ✓
server.runti..prod.js gzip 51.5 kB 51.5 kB ✓
Overall change 446 kB 446 kB ✓
build cache Overall increase ⚠️
vercel/next.js 14-2-1 vercel/next.js zt/cherrypick-67358 Change
0.pack gzip 1.6 MB 1.6 MB ⚠️ +326 B
index.pack gzip 113 kB 113 kB N/A
Overall change 1.6 MB 1.6 MB ⚠️ +326 B
Diff details
Diff for edge-ssr.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

Commit: 929cb69

@ztanner ztanner marked this pull request as ready for review September 30, 2024 13:58
@ztanner ztanner merged commit 50e41a2 into 14-2-1 Sep 30, 2024
50 of 59 checks passed
@ztanner ztanner deleted the zt/cherrypick-67358 branch September 30, 2024 16:13
@ztanner ztanner mentioned this pull request Sep 30, 2024
Closed
zemnmez-renovate-bot added a commit to zemn-me/monorepo that referenced this pull request Oct 1, 2024
@zemnmez-renovate-bot
fix(deps): update nextjs monorepo to v14.2.14
3c1699d 
##### [`v14.2.14](https://github.com/vercel/next.js/releases/tag/v14.2.14)

> \[!NOTE]\
> This release is backporting bug fixes. It does **not** include all pending features/changes on canary.

##### Core Changes

-   Fix: clone response in first handler to prevent race ([#70082](vercel/next.js#70082)) ([#70649](vercel/next.js#70649))
-   Respect reexports from metadata API routes ([#70508](vercel/next.js#70508)) ([#70647](vercel/next.js#70647))
-   Externalize node binary modules for app router ([#70646](vercel/next.js#70646))
-   Fix revalidateTag() behaviour when invoked in server components ([#70446](vercel/next.js#70446)) ([#70642](vercel/next.js#70642))
-   Fix prefetch bailout detection for nested loading segments ([#70618](vercel/next.js#70618))
-   Add missing node modules to externals ([#70382](vercel/next.js#70382))
-   Feature: next/image: add support for images.remotePatterns.search ([#70302](vercel/next.js#70302))

##### Credits

Huge thanks to [@styfle](https://github.com/styfle), [@ztanner](https://github.com/ztanner), [@ijjk](https://github.com/ijjk), [@huozhi](https://github.com/huozhi) and [@wyattjoh](https://github.com/wyattjoh) for helping!
github-merge-queue bot pushed a commit to zemn-me/monorepo that referenced this pull request Oct 1, 2024
@zemnmez-renovate-bot
fix(deps): update nextjs monorepo to v14.2.14
cc9306c 
##### [`v14.2.14](https://github.com/vercel/next.js/releases/tag/v14.2.14)

> \[!NOTE]\
> This release is backporting bug fixes. It does **not** include all pending features/changes on canary.

##### Core Changes

-   Fix: clone response in first handler to prevent race ([#70082](vercel/next.js#70082)) ([#70649](vercel/next.js#70649))
-   Respect reexports from metadata API routes ([#70508](vercel/next.js#70508)) ([#70647](vercel/next.js#70647))
-   Externalize node binary modules for app router ([#70646](vercel/next.js#70646))
-   Fix revalidateTag() behaviour when invoked in server components ([#70446](vercel/next.js#70446)) ([#70642](vercel/next.js#70642))
-   Fix prefetch bailout detection for nested loading segments ([#70618](vercel/next.js#70618))
-   Add missing node modules to externals ([#70382](vercel/next.js#70382))
-   Feature: next/image: add support for images.remotePatterns.search ([#70302](vercel/next.js#70302))

##### Credits

Huge thanks to [@styfle](https://github.com/styfle), [@ztanner](https://github.com/ztanner), [@ijjk](https://github.com/ijjk), [@huozhi](https://github.com/huozhi) and [@wyattjoh](https://github.com/wyattjoh) for helping!
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Oct 1, 2024
@renovate
fix(deps): update dependency @next/eslint-plugin-next to v14.2.14
7e7121e 
##### [v14.2.14](https://github.com/vercel/next.js/releases/tag/v14.2.14)

> \[!NOTE]\
> This release is backporting bug fixes. It does **not** include all pending features/changes on canary.

##### Core Changes

-   Fix: clone response in first handler to prevent race ([#70082](vercel/next.js#70082)) ([#70649](vercel/next.js#70649))
-   Respect reexports from metadata API routes ([#70508](vercel/next.js#70508)) ([#70647](vercel/next.js#70647))
-   Externalize node binary modules for app router ([#70646](vercel/next.js#70646))
-   Fix revalidateTag() behaviour when invoked in server components ([#70446](vercel/next.js#70446)) ([#70642](vercel/next.js#70642))
-   Fix prefetch bailout detection for nested loading segments ([#70618](vercel/next.js#70618))
-   Add missing node modules to externals ([#70382](vercel/next.js#70382))
-   Feature: next/image: add support for images.remotePatterns.search ([#70302](vercel/next.js#70302))

##### Credits

Huge thanks to [@styfle](https://github.com/styfle), [@ztanner](https://github.com/ztanner), [@ijjk](https://github.com/ijjk), [@huozhi](https://github.com/huozhi) and [@wyattjoh](https://github.com/wyattjoh) for helping!
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Oct 1, 2024
@renovate
fix(deps): update dependency @next/eslint-plugin-next to v14.2.14
f0a7175 
##### [v14.2.14](https://github.com/vercel/next.js/releases/tag/v14.2.14)

> \[!NOTE]\
> This release is backporting bug fixes. It does **not** include all pending features/changes on canary.

##### Core Changes

-   Fix: clone response in first handler to prevent race ([#70082](vercel/next.js#70082)) ([#70649](vercel/next.js#70649))
-   Respect reexports from metadata API routes ([#70508](vercel/next.js#70508)) ([#70647](vercel/next.js#70647))
-   Externalize node binary modules for app router ([#70646](vercel/next.js#70646))
-   Fix revalidateTag() behaviour when invoked in server components ([#70446](vercel/next.js#70446)) ([#70642](vercel/next.js#70642))
-   Fix prefetch bailout detection for nested loading segments ([#70618](vercel/next.js#70618))
-   Add missing node modules to externals ([#70382](vercel/next.js#70382))
-   Feature: next/image: add support for images.remotePatterns.search ([#70302](vercel/next.js#70302))

##### Credits

Huge thanks to [@styfle](https://github.com/styfle), [@ztanner](https://github.com/ztanner), [@ijjk](https://github.com/ijjk), [@huozhi](https://github.com/huozhi) and [@wyattjoh](https://github.com/wyattjoh) for helping!
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ijjk ijjk ijjk approved these changes

@huozhi huozhi Awaiting requested review from huozhi

@eps1lon eps1lon Awaiting requested review from eps1lon

@lubieowoce lubieowoce Awaiting requested review from lubieowoce

Assignees
No one assigned
Labels
created-by: Next.js team PRs by the Next.js team. locked tests type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ztanner @ijjk