Skip to content

Commit

Permalink
Remove CacheNode.status field
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Dec 11, 2023
1 parent e02fe31 commit b47a40b
Show file tree
Hide file tree
Showing 24 changed files with 53 additions and 271 deletions.
2 changes: 0 additions & 2 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
AppRouterContext,
LayoutRouterContext,
GlobalLayoutRouterContext,
CacheStates,
} from '../../shared/lib/app-router-context.shared-runtime'
import type {
CacheNode,
Expand Down Expand Up @@ -149,7 +148,6 @@ function HistoryUpdater({
}

export const createEmptyCacheNode = () => ({
status: CacheStates.LAZY_INITIALIZED,
data: null,
subTreeData: null,
parallelRoutes: new Map(),
Expand Down
14 changes: 3 additions & 11 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type { FocusAndScrollRef } from './router-reducer/router-reducer-types'
import React, { useContext, use, startTransition, Suspense } from 'react'
import ReactDOM from 'react-dom'
import {
CacheStates,
LayoutRouterContext,
GlobalLayoutRouterContext,
TemplateContext,
Expand Down Expand Up @@ -339,30 +338,23 @@ function InnerLayoutRouter({
let childNode = childNodes.get(cacheKey)

// When childNode is not available during rendering client-side we need to fetch it from the server.
if (!childNode || childNode.status === CacheStates.LAZY_INITIALIZED) {
if (!childNode || childNode.subTreeData === null) {
/**
* Router state with refetch marker added
*/
// TODO-APP: remove ''
const refetchTree = walkAddRefetch(['', ...segmentPath], fullTree)

childNode = {
status: CacheStates.DATA_FETCH,
data: fetchServerResponse(
new URL(url, location.origin),
refetchTree,
context.nextUrl,
buildId
),
subTreeData: null,
head:
childNode && childNode.status === CacheStates.LAZY_INITIALIZED
? childNode.head
: undefined,
parallelRoutes:
childNode && childNode.status === CacheStates.LAZY_INITIALIZED
? childNode.parallelRoutes
: new Map(),
head: childNode ? childNode.head : undefined,
parallelRoutes: childNode ? childNode.parallelRoutes : new Map(),
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import type { FlightDataPath } from '../../../server/app-render/types'
import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head'
Expand All @@ -20,7 +19,6 @@ export function applyFlightData(

if (flightDataPath.length === 3) {
const subTreeData = cacheNodeSeedData[2]
cache.status = CacheStates.READY
cache.subTreeData = subTreeData
fillLazyItemsTillLeafWithHead(
cache,
Expand All @@ -32,7 +30,6 @@ export function applyFlightData(
)
} else {
// Copy subTreeData for the root node of the cache.
cache.status = CacheStates.READY
cache.subTreeData = existingCache.subTreeData
cache.parallelRoutes = new Map(existingCache.parallelRoutes)
// Create a copy of the existing cache with the subTreeData applied.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react'
import type { FlightRouterState } from '../../../server/app-render/types'
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import { createInitialRouterState } from './create-initial-router-state'

Expand Down Expand Up @@ -56,7 +55,6 @@ describe('createInitialRouterState', () => {
})

const expectedCache: CacheNode = {
status: CacheStates.READY,
data: null,
subTreeData: children,
parallelRoutes: new Map([
Expand All @@ -66,15 +64,13 @@ describe('createInitialRouterState', () => {
[
'linking',
{
status: CacheStates.LAZY_INITIALIZED,
parallelRoutes: new Map([
[
'children',
new Map([
[
'',
{
status: CacheStates.LAZY_INITIALIZED,
data: null,
subTreeData: null,
parallelRoutes: new Map(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type {
CacheNodeSeedData,
} from '../../../server/app-render/types'

import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import { createHrefFromUrl } from './create-href-from-url'
import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head'
import { extractPathFromFlightRouterState } from './compute-changed-path'
Expand Down Expand Up @@ -34,7 +33,6 @@ export function createInitialRouterState({
const subTreeData = initialSeedData[2]

const cache: CacheNode = {
status: CacheStates.READY,
data: null,
subTreeData: subTreeData,
// The cache gets seeded during the first render. `initialParallelRoutes` ensures the cache from the first render is there during the second render.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react'
import type { FetchServerResponseResult } from './fetch-server-response'
import { fillCacheWithDataProperty } from './fill-cache-with-data-property'
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'

describe('fillCacheWithDataProperty', () => {
Expand All @@ -23,14 +22,12 @@ describe('fillCacheWithDataProperty', () => {
.flat()

const cache: CacheNode = {
status: CacheStates.LAZY_INITIALIZED,
data: null,
subTreeData: null,
parallelRoutes: new Map(),
}
const existingCache: CacheNode = {
data: null,
status: CacheStates.READY,
subTreeData: <>Root layout</>,
parallelRoutes: new Map([
[
Expand All @@ -40,7 +37,6 @@ describe('fillCacheWithDataProperty', () => {
'linking',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Linking</>,
parallelRoutes: new Map([
[
Expand All @@ -50,7 +46,6 @@ describe('fillCacheWithDataProperty', () => {
'',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Page</>,
parallelRoutes: new Map(),
},
Expand Down Expand Up @@ -81,27 +76,23 @@ describe('fillCacheWithDataProperty', () => {
"" => {
"data": null,
"parallelRoutes": Map {},
"status": "READY",
"subTreeData": <React.Fragment>
Page
</React.Fragment>,
},
},
},
"status": "READY",
"subTreeData": <React.Fragment>
Linking
</React.Fragment>,
},
"dashboard" => {
"data": Promise {},
"parallelRoutes": Map {},
"status": "DATAFETCH",
"subTreeData": null,
},
},
},
"status": "LAZYINITIALIZED",
"subTreeData": null,
}
`)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { FetchServerResponseResult } from './fetch-server-response'
import type { FlightSegmentPath } from '../../../server/app-render/types'
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import { createRouterCacheKey } from './create-router-cache-key'

Expand Down Expand Up @@ -39,7 +38,6 @@ export function fillCacheWithDataProperty(
childCacheNode === existingChildCacheNode
) {
childSegmentMap.set(cacheKey, {
status: CacheStates.DATA_FETCH,
data: fetchResponse(),
subTreeData: null,
parallelRoutes: new Map(),
Expand All @@ -52,7 +50,6 @@ export function fillCacheWithDataProperty(
// Start fetch in the place where the existing cache doesn't have the data yet.
if (!childCacheNode) {
childSegmentMap.set(cacheKey, {
status: CacheStates.DATA_FETCH,
data: fetchResponse(),
subTreeData: null,
parallelRoutes: new Map(),
Expand All @@ -63,7 +60,6 @@ export function fillCacheWithDataProperty(

if (childCacheNode === existingChildCacheNode) {
childCacheNode = {
status: childCacheNode.status,
data: childCacheNode.data,
subTreeData: childCacheNode.subTreeData,
parallelRoutes: new Map(childCacheNode.parallelRoutes),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react'
import { fillCacheWithNewSubTreeData } from './fill-cache-with-new-subtree-data'
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import type { FlightData } from '../../../server/app-render/types'

Expand Down Expand Up @@ -28,14 +27,12 @@ const getFlightData = (): FlightData => {
describe('fillCacheWithNewSubtreeData', () => {
it('should apply subTreeData and head property', () => {
const cache: CacheNode = {
status: CacheStates.LAZY_INITIALIZED,
data: null,
subTreeData: null,
parallelRoutes: new Map(),
}
const existingCache: CacheNode = {
data: null,
status: CacheStates.READY,
subTreeData: <>Root layout</>,
parallelRoutes: new Map([
[
Expand All @@ -45,7 +42,6 @@ describe('fillCacheWithNewSubtreeData', () => {
'linking',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Linking</>,
parallelRoutes: new Map([
[
Expand All @@ -55,7 +51,6 @@ describe('fillCacheWithNewSubtreeData', () => {
'',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Page</>,
parallelRoutes: new Map(),
},
Expand Down Expand Up @@ -83,7 +78,6 @@ describe('fillCacheWithNewSubtreeData', () => {

const expectedCache: CacheNode = {
data: null,
status: CacheStates.LAZY_INITIALIZED,
subTreeData: null,
parallelRoutes: new Map([
[
Expand All @@ -93,7 +87,6 @@ describe('fillCacheWithNewSubtreeData', () => {
'linking',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Linking</>,
parallelRoutes: new Map([
[
Expand All @@ -104,7 +97,6 @@ describe('fillCacheWithNewSubtreeData', () => {
'',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Page</>,
parallelRoutes: new Map(),
},
Expand All @@ -121,7 +113,6 @@ describe('fillCacheWithNewSubtreeData', () => {
'',
{
data: null,
status: CacheStates.LAZY_INITIALIZED,
subTreeData: null,
parallelRoutes: new Map(),
head: (
Expand All @@ -135,7 +126,6 @@ describe('fillCacheWithNewSubtreeData', () => {
],
]),
subTreeData: <h1>SubTreeData Injected!</h1>,
status: CacheStates.READY,
},
],
]),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import type {
FlightDataPath,
Expand Down Expand Up @@ -49,7 +48,6 @@ export function fillCacheWithNewSubTreeData(
const seedData: CacheNodeSeedData = flightDataPath[3]
const subTreeData = seedData[2]
childCacheNode = {
status: CacheStates.READY,
data: null,
subTreeData,
// Ensure segments other than the one we got data for are preserved.
Expand Down Expand Up @@ -88,7 +86,6 @@ export function fillCacheWithNewSubTreeData(

if (childCacheNode === existingChildCacheNode) {
childCacheNode = {
status: childCacheNode.status,
data: childCacheNode.data,
subTreeData: childCacheNode.subTreeData,
parallelRoutes: new Map(childCacheNode.parallelRoutes),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react'
import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head'
import { CacheStates } from '../../../shared/lib/app-router-context.shared-runtime'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import type { FlightData } from '../../../server/app-render/types'

Expand Down Expand Up @@ -37,14 +36,12 @@ const getFlightData = (): FlightData => {
describe('fillLazyItemsTillLeafWithHead', () => {
it('should fill lazy items till leaf with head', () => {
const cache: CacheNode = {
status: CacheStates.LAZY_INITIALIZED,
data: null,
subTreeData: null,
parallelRoutes: new Map(),
}
const existingCache: CacheNode = {
data: null,
status: CacheStates.READY,
subTreeData: <>Root layout</>,
parallelRoutes: new Map([
[
Expand All @@ -54,7 +51,6 @@ describe('fillLazyItemsTillLeafWithHead', () => {
'linking',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Linking</>,
parallelRoutes: new Map([
[
Expand All @@ -64,7 +60,6 @@ describe('fillLazyItemsTillLeafWithHead', () => {
'',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Page</>,
parallelRoutes: new Map(),
},
Expand Down Expand Up @@ -98,7 +93,6 @@ describe('fillLazyItemsTillLeafWithHead', () => {

const expectedCache: CacheNode = {
data: null,
status: CacheStates.LAZY_INITIALIZED,
subTreeData: null,
parallelRoutes: new Map([
[
Expand All @@ -108,7 +102,6 @@ describe('fillLazyItemsTillLeafWithHead', () => {
'linking',
{
data: null,
status: CacheStates.LAZY_INITIALIZED,
subTreeData: null,
parallelRoutes: new Map([
[
Expand All @@ -126,7 +119,6 @@ describe('fillLazyItemsTillLeafWithHead', () => {
'',
{
data: null,
status: CacheStates.LAZY_INITIALIZED,
subTreeData: null,
parallelRoutes: new Map(),
head: (
Expand All @@ -140,14 +132,12 @@ describe('fillLazyItemsTillLeafWithHead', () => {
],
]),
subTreeData: null,
status: CacheStates.LAZY_INITIALIZED,
},
],
[
'',
{
data: null,
status: CacheStates.READY,
subTreeData: <>Page</>,
parallelRoutes: new Map(),
},
Expand Down

0 comments on commit b47a40b

Please sign in to comment.