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
Remove CacheNode.status field #59472
Conversation
Tests Passed |
cf00aa3
to
b47a40b
Compare
Stats from current PRDefault BuildGeneral
Client Bundles (main, webpack)
Legacy Client Bundles (polyfills)
Client Pages
Client Build Manifests
Rendered Page Sizes
Edge SSR bundle Size
Middleware size
Next Runtimes
Diff detailsDiff for page.jsDiff too large to display Diff for edge-ssr.jsDiff too large to display Diff for 170-HASH.jsDiff too large to display Diff for main-HASH.jsDiff too large to display Diff for app-page-exp..ntime.dev.jsfailed to diff Diff for app-page-exp..time.prod.jsDiff too large to display Diff for app-page-tur..time.prod.jsDiff too large to display Diff for app-page-tur..time.prod.jsDiff too large to display Diff for app-page.runtime.dev.jsDiff too large to display Diff for app-page.runtime.prod.jsDiff too large to display Diff for app-route-ex..ntime.dev.jsDiff too large to display Diff for app-route-ex..time.prod.jsDiff too large to display Diff for app-route-tu..time.prod.jsDiff too large to display Diff for app-route-tu..time.prod.jsDiff too large to display Diff for app-route.runtime.dev.jsDiff too large to display Diff for app-route.ru..time.prod.jsDiff too large to display Diff for pages-turbo...time.prod.jsDiff too large to display Diff for pages.runtime.dev.jsDiff too large to display Diff for pages.runtime.prod.jsDiff too large to display |
b47a40b
to
4fc6792
Compare
I'm about to make some changes to the CacheNode data structure, and before I add more complexity, I noticed an opportunity to remove some — the `status` field isn't logically necessary: - The `DATA_FETCH` and `LAZY_INITIALIZED` states are already treated as equivalent; in either case, they will cause the render to suspend during render, trigger a lazy data fetch (if one hasn't been triggered already), and then update the router with the result of the response. - `subTreeData` is null if and only if the node is in the `DATA_FETCH` or `LAZY_INITIALIZED` states, and it always causes the render to suspend. So rather than check if the status is one of those, we can check if `subTreeData` is null. The most important changes are to CacheNode type in app-router-context.shared-runtime and the lazy fetching logic in LayoutRouter. Everything else in the diff is related to deleting the `status` field wherever a CacheNode is referenced, like in the reducer unit tests.
4fc6792
to
f4206e8
Compare
I'm about to make some changes to the CacheNode data structure, and before I add more complexity, I noticed an opportunity to remove some — the
status
field isn't logically necessary:DATA_FETCH
andLAZY_INITIALIZED
states are already treated as equivalent; in either case, they will cause the render to suspend during render, trigger a lazy data fetch (if one hasn't been triggered already), and then update the router with the result of the response.subTreeData
is null if and only if the node is in theDATA_FETCH
orLAZY_INITIALIZED
states, and it always causes the render to suspend. So rather than check if the status is one of those, we can check ifsubTreeData
is null.The most important changes are to CacheNode type in app-router-context.shared-runtime and the lazy fetching logic in LayoutRouter. Everything else in the diff is related to deleting the
status
field wherever a CacheNode is referenced, like in the reducer unit tests.Closes NEXT-1842