Skip to content

Commit

Permalink
Revert "Reland app-router: new client-side cache semantics" (#48688)
Browse files Browse the repository at this point in the history
Reverts #48685

Temporary Revert again to investigate the hang job
fix NEXT-1011
  • Loading branch information
huozhi committed Apr 21, 2023
1 parent b61305a commit 8089d0a
Show file tree
Hide file tree
Showing 27 changed files with 271 additions and 951 deletions.
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

0 comments on commit 8089d0a

Please sign in to comment.