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

Revert "Reland app-router: new client-side cache semantics" #48688

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
ACTION_REFRESH,
ACTION_RESTORE,
ACTION_SERVER_PATCH,
PrefetchKind,
} from './router-reducer/router-reducer-types'
import { createHrefFromUrl } from './router-reducer/create-href-from-url'
import {
Expand Down Expand Up @@ -235,7 +234,7 @@ function Router({
const routerInstance: AppRouterInstance = {
back: () => window.history.back(),
forward: () => window.history.forward(),
prefetch: async (href, options) => {
prefetch: async (href) => {
// If prefetch has already been triggered, don't trigger it again.
if (isBot(window.navigator.userAgent)) {
return
Expand All @@ -245,12 +244,12 @@ function Router({
if (isExternalURL(url)) {
return
}

// @ts-ignore startTransition exists
React.startTransition(() => {
dispatch({
type: ACTION_PREFETCH,
url,
kind: options?.kind ?? PrefetchKind.FULL,
})
})
},
Expand Down
20 changes: 12 additions & 8 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,16 @@ function InnerLayoutRouter({
// TODO-APP: verify if this can be null based on user code
childProp.current !== null
) {
if (!childNode) {
if (childNode) {
if (childNode.status === CacheStates.LAZY_INITIALIZED) {
// @ts-expect-error we're changing it's type!
childNode.status = CacheStates.READY
// @ts-expect-error
childNode.subTreeData = childProp.current
// Mutates the prop in order to clean up the memory associated with the subTreeData as it is now part of the cache.
childProp.current = null
}
} else {
// Add the segment's subTreeData to the cache.
// This writes to the cache when there is no item in the cache yet. It never *overwrites* existing cache items which is why it's safe in concurrent mode.
childNodes.set(cacheKey, {
Expand All @@ -286,15 +295,10 @@ function InnerLayoutRouter({
subTreeData: childProp.current,
parallelRoutes: new Map(),
})
// Mutates the prop in order to clean up the memory associated with the subTreeData as it is now part of the cache.
childProp.current = null
// In the above case childNode was set on childNodes, so we have to get it from the cacheNodes again.
childNode = childNodes.get(cacheKey)
} else {
if (childNode.status === CacheStates.LAZY_INITIALIZED) {
// @ts-expect-error we're changing it's type!
childNode.status = CacheStates.READY
// @ts-expect-error
childNode.subTreeData = childProp.current
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export function applyFlightData(
existingCache: CacheNode,
cache: CacheNode,
flightDataPath: FlightDataPath,
wasPrefetched: boolean = false
wasPrefetched?: boolean
): boolean {
// The one before last item is the router state tree patch
const [treePatch, subTreeData, head] = flightDataPath.slice(-3)
Expand All @@ -33,12 +33,7 @@ export function applyFlightData(
cache.subTreeData = existingCache.subTreeData
cache.parallelRoutes = new Map(existingCache.parallelRoutes)
// Create a copy of the existing cache with the subTreeData applied.
fillCacheWithNewSubTreeData(
cache,
existingCache,
flightDataPath,
wasPrefetched
)
fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath)
}

return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
} from '../app-router-headers'
import { urlToUrlWithoutFlightMarker } from '../app-router'
import { callServer } from '../../app-call-server'
import { PrefetchKind } from './router-reducer-types'

/**
* Fetch the flight data for the provided url. Takes in the current router state to decide what to render server-side.
Expand All @@ -24,7 +23,7 @@ export async function fetchServerResponse(
url: URL,
flightRouterState: FlightRouterState,
nextUrl: string | null,
prefetchKind?: PrefetchKind
prefetch?: true
): Promise<[FlightData: FlightData, canonicalUrlOverride: URL | undefined]> {
const headers: {
[RSC]: '1'
Expand All @@ -37,14 +36,8 @@ export async function fetchServerResponse(
// Provide the current router state
[NEXT_ROUTER_STATE_TREE]: JSON.stringify(flightRouterState),
}

/**
* Three cases:
* - `prefetchKind` is `undefined`, it means it's a normal navigation, so we want to prefetch the page data fully
* - `prefetchKind` is `full` - we want to prefetch the whole page so same as above
* - `prefetchKind` is `auto` - if the page is dynamic, prefetch the page data partially, if static prefetch the page data fully
*/
if (prefetchKind === PrefetchKind.AUTO) {
if (prefetch) {
// Enable prefetch response
headers[NEXT_ROUTER_PREFETCH] = '1'
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { FlightSegmentPath } from '../../../server/app-render/types'
import { CacheNode, CacheStates } from '../../../shared/lib/app-router-context'
import { createRouterCacheKey } from './create-router-cache-key'
import { fetchServerResponse } from './fetch-server-response'

/**
Expand All @@ -9,24 +7,19 @@ import { fetchServerResponse } from './fetch-server-response'
export function fillCacheWithDataProperty(
newCache: CacheNode,
existingCache: CacheNode,
flightSegmentPath: FlightSegmentPath,
fetchResponse: () => ReturnType<typeof fetchServerResponse>,
bailOnParallelRoutes: boolean = false
segments: string[],
fetchResponse: () => ReturnType<typeof fetchServerResponse>
): { bailOptimistic: boolean } | undefined {
const isLastEntry = flightSegmentPath.length <= 2
const isLastEntry = segments.length === 1

const [parallelRouteKey, segment] = flightSegmentPath
const cacheKey = createRouterCacheKey(segment)
const parallelRouteKey = 'children'
const [segment] = segments

const existingChildSegmentMap =
existingCache.parallelRoutes.get(parallelRouteKey)

if (
!existingChildSegmentMap ||
(bailOnParallelRoutes && existingCache.parallelRoutes.size > 1)
) {
if (!existingChildSegmentMap) {
// Bailout because the existing cache does not have the path to the leaf node
// or the existing cache has multiple parallel routes
// Will trigger lazy fetch in layout-router because of missing segment
return { bailOptimistic: true }
}
Expand All @@ -38,8 +31,8 @@ export function fillCacheWithDataProperty(
newCache.parallelRoutes.set(parallelRouteKey, childSegmentMap)
}

const existingChildCacheNode = existingChildSegmentMap.get(cacheKey)
let childCacheNode = childSegmentMap.get(cacheKey)
const existingChildCacheNode = existingChildSegmentMap.get(segment)
let childCacheNode = childSegmentMap.get(segment)

// In case of last segment start off the fetch at this level and don't copy further down.
if (isLastEntry) {
Expand All @@ -48,7 +41,7 @@ export function fillCacheWithDataProperty(
!childCacheNode.data ||
childCacheNode === existingChildCacheNode
) {
childSegmentMap.set(cacheKey, {
childSegmentMap.set(segment, {
status: CacheStates.DATA_FETCH,
data: fetchResponse(),
subTreeData: null,
Expand All @@ -61,7 +54,7 @@ export function fillCacheWithDataProperty(
if (!childCacheNode || !existingChildCacheNode) {
// Start fetch in the place where the existing cache doesn't have the data yet.
if (!childCacheNode) {
childSegmentMap.set(cacheKey, {
childSegmentMap.set(segment, {
status: CacheStates.DATA_FETCH,
data: fetchResponse(),
subTreeData: null,
Expand All @@ -78,13 +71,13 @@ export function fillCacheWithDataProperty(
subTreeData: childCacheNode.subTreeData,
parallelRoutes: new Map(childCacheNode.parallelRoutes),
} as CacheNode
childSegmentMap.set(cacheKey, childCacheNode)
childSegmentMap.set(segment, childCacheNode)
}

return fillCacheWithDataProperty(
childCacheNode,
existingChildCacheNode,
flightSegmentPath.slice(2),
segments.slice(1),
fetchResponse
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('fillCacheWithNewSubtreeData', () => {
// Mirrors the way router-reducer values are passed in.
const flightDataPath = flightData[0]

fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath, false)
fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath)

const expectedCache: CacheNode = {
data: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { createRouterCacheKey } from './create-router-cache-key'
export function fillCacheWithNewSubTreeData(
newCache: CacheNode,
existingCache: CacheNode,
flightDataPath: FlightDataPath,
wasPrefetched?: boolean
flightDataPath: FlightDataPath
): void {
const isLastEntry = flightDataPath.length <= 5
const [parallelRouteKey, segment] = flightDataPath
Expand Down Expand Up @@ -64,8 +63,7 @@ export function fillCacheWithNewSubTreeData(
childCacheNode,
existingChildCacheNode,
flightDataPath[2],
flightDataPath[4],
wasPrefetched
flightDataPath[4]
)

childSegmentMap.set(cacheKey, childCacheNode)
Expand All @@ -92,7 +90,6 @@ export function fillCacheWithNewSubTreeData(
fillCacheWithNewSubTreeData(
childCacheNode,
existingChildCacheNode,
flightDataPath.slice(2),
wasPrefetched
flightDataPath.slice(2)
)
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('invalidateCacheBelowFlightSegmentPath', () => {
// @ts-expect-error TODO-APP: investigate why this is not a TS error in router-reducer.
cache.subTreeData = existingCache.subTreeData
// Create a copy of the existing cache with the subTreeData applied.
fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath, false)
fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath)

// Invalidate the cache below the flight segment path. This should remove the 'about' node.
invalidateCacheBelowFlightSegmentPath(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ import {
ACTION_NAVIGATE,
ACTION_PREFETCH,
PrefetchAction,
PrefetchKind,
} from '../router-reducer-types'
import { navigateReducer } from './navigate-reducer'
import { prefetchReducer } from './prefetch-reducer'
Expand Down Expand Up @@ -1006,7 +1005,6 @@ describe('navigateReducer', () => {
const prefetchAction: PrefetchAction = {
type: ACTION_PREFETCH,
url,
kind: PrefetchKind.AUTO,
}

const state = createInitialRouterState({
Expand Down Expand Up @@ -1088,9 +1086,6 @@ describe('navigateReducer', () => {
'/linking/about',
{
data: record,
kind: PrefetchKind.AUTO,
lastUsedTime: null,
prefetchTime: expect.any(Number),
treeAtTimeOfPrefetch: [
'',
{
Expand Down
Loading