Store loading data in CacheNode #62346
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Feb 21, 2024
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
2 times, most recently
from
372be9e
to
20eb139
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
3 times, most recently
from
b0fa16c
to
bed24f1
Compare
Tests Passed |
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
2 times, most recently
from
296e046
to
eb48100
Compare
Stats from current PRDefault Build (Increase detected
|
| vercel/next.js canary | vercel/next.js 02-21-Store_loading_data_in_CacheNode | Change | |
|---|---|---|---|
| buildDuration | 13.8s | 13.6s | N/A |
| buildDurationCached | 7.4s | 6.2s | N/A |
| nodeModulesSize | 198 MB | 198 MB | |
| nextStartRea..uration (ms) | 429ms | 438ms | N/A |
Client Bundles (main, webpack)
| vercel/next.js canary | vercel/next.js 02-21-Store_loading_data_in_CacheNode | Change | |
|---|---|---|---|
| 2453-HASH.js gzip | 30.4 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.2 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 02-21-Store_loading_data_in_CacheNode | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | vercel/next.js 02-21-Store_loading_data_in_CacheNode | 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 02-21-Store_loading_data_in_CacheNode | 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 02-21-Store_loading_data_in_CacheNode | Change | |
|---|---|---|---|
| index.html gzip | 530 B | 527 B | N/A |
| link.html gzip | 541 B | 540 B | N/A |
| withRouter.html gzip | 525 B | 522 B | N/A |
| Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
| vercel/next.js canary | vercel/next.js 02-21-Store_loading_data_in_CacheNode | 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 02-21-Store_loading_data_in_CacheNode | 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 02-21-Store_loading_data_in_CacheNode | Change | |
|---|---|---|---|
| app-page-exp...dev.js gzip | 171 kB | 171 kB | N/A |
| app-page-exp..prod.js gzip | 96.6 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 | 149 kB | 149 kB | N/A |
| app-page.run..prod.js gzip | 91.3 kB | 91.3 kB | N/A |
| 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 | 249 kB | 249 kB | ✓ |
build cache Overall increase ⚠️
| vercel/next.js canary | vercel/next.js 02-21-Store_loading_data_in_CacheNode | Change | |
|---|---|---|---|
| 0.pack gzip | 1.55 MB | 1.56 MB | |
| index.pack gzip | 105 kB | 105 kB | N/A |
| Overall change | 1.55 MB | 1.56 MB |
Diff details
Diff for middleware.js
Diff too large to display
Diff for 2453-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
failed to diffDiff for app-page.runtime.prod.js
Diff too large to display
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
from
20eb139
to
b735632
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
from
eb48100
to
1c7b81e
Compare
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
from
b735632
to
c09003d
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
3 times, most recently
from
810c85a
to
db8eaaf
Compare
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
from
c09003d
to
a4761d8
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
from
db8eaaf
to
22c00ad
Compare
ztanner
requested review from
timneutkens,
ijjk,
shuding,
huozhi and
feedthejim
as code owners
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
from
a4761d8
to
42cbd36
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
from
22c00ad
to
edccdea
Compare
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
from
42cbd36
to
fbf9831
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
2 times, most recently
from
ff8cb80
to
4f7e581
Compare
ztanner
force-pushed
the
02-21-Pass_whole_prefetch_entry_rather_than_status_property
branch
from
fbf9831
to
be82a9d
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
from
4f7e581
to
3540b25
Compare
Base automatically changed from
02-21-Pass_whole_prefetch_entry_rather_than_status_property
to
canary
March 7, 2024 01:14
ijjk
approved these changes
Mar 7, 2024
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
2 times, most recently
from
4d9b44f
to
1be6d1b
Compare
ztanner
force-pushed
the
02-21-Store_loading_data_in_CacheNode
branch
from
1be6d1b
to
9c738c3
Compare
acdlite
approved these changes
Mar 14, 2024
ztanner
added a commit
that referenced
this pull request
Mar 19, 2024
In #61573, I updated the navigation reducer to request a new prefetch entry if it's stale. But this has the unintended consequence of making instant loading states effectively useless after 30s (when the prefetch would have expired). Blocking navigation and then rendering the loading state isn't ideal - if we have some loading data in a cache node, we should re-use it. Now that #62346 stores loading data in the `CacheNode`, we can copy over `loading` during a navigation. This PR repurposes `fillCacheWithDataProperty` which wasn't being used anywhere, to instead be a utility we can use to programmatically trigger a lazy fetch on a particular segment path by nulling out it's data while copying over other properties. We could have used the existing util as-is, but ideally we only have a single spot where lazy fetching can happen, which currently is in `LayoutRouter`. When a stale prefetch entry is detected, rather than applying the data to the tree, this PR will copy over the `loading` nodes and will "delete" the data so it can be refetched. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-2806
ztanner
added a commit
that referenced
this pull request
Mar 26, 2024
This ensures that even if a `loading.js` returns `null`, that we still render a `Suspense` boundary, as it's perfectly valid to have an empty fallback. This was accidentally lost in #62346 -- this brings back the `hasLoading` prop which will check the loading module itself (rather than the `ReactNode`) for truthiness, and I've added a test to avoid another regression. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-2936
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes how loading components are provided to
LayoutRouterto instead be part of theCacheNode. This makes it so that we can copy it over/expire it in a similar way to thedataproperty on aCacheNode. Consequently, this fixes a bug in PPR navigations, where 2 different loading states (the prefetched one, followed by the updated one) might be displayed upon navigating.Note: This disables a PPR test that asserts the loading data gets expired after 5 minutes. As implemented, the loading data will remain cached for a particular segment.
Closes NEXT-2557