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

app-router: new client-side cache semantics #48383

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
78fa2f5
initial changes
feedthejim Apr 14, 2023
2391805
cleanup
feedthejim Apr 14, 2023
d788576
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 17, 2023
9b1c195
use string enums
feedthejim Apr 17, 2023
0e6e717
change comment
feedthejim Apr 17, 2023
db0ac92
prune cache entries + create temp cache entries on router.push
feedthejim Apr 17, 2023
6f74db1
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 17, 2023
4fe7c81
remove tests
feedthejim Apr 17, 2023
652a7a6
add new methods to the browser interface
feedthejim Apr 19, 2023
b09f0bc
add tests
feedthejim Apr 19, 2023
4fd6eed
final implementation
feedthejim Apr 19, 2023
d5f99cf
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 19, 2023
a90421c
disable lint
feedthejim Apr 19, 2023
a2a5f84
fix scroll
feedthejim Apr 19, 2023
7989fdd
remove tests
feedthejim Apr 19, 2023
bbf6e3d
test out removing the optimistic nav path
feedthejim Apr 19, 2023
f74255b
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 19, 2023
4334b22
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 19, 2023
577ce1f
add a new fresh
feedthejim Apr 20, 2023
84d8bbc
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 20, 2023
7c6d769
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 21, 2023
65e533d
remove inferred param
feedthejim Apr 21, 2023
6f8705d
prune prefetch cache on prefetch
feedthejim Apr 21, 2023
85c3c15
refactor logic in layout-router
feedthejim Apr 21, 2023
66e66fe
use enums for prefetch kind
feedthejim Apr 21, 2023
4b540ae
navigate refactor
feedthejim Apr 21, 2023
95ef6cf
fix optimisticNav
feedthejim Apr 21, 2023
42d3679
fix build
feedthejim Apr 21, 2023
b52dc80
don't write to cache.data
feedthejim Apr 21, 2023
4500047
fix optimistic segment
feedthejim Apr 21, 2023
0c817ee
fix enum usage
feedthejim Apr 21, 2023
ebf08f0
Merge branch 'canary' into feedthejim/next-1011-implement-new-caching…
feedthejim Apr 21, 2023
afb3f7f
fix flaky test
feedthejim Apr 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ function Router({
const routerInstance: AppRouterInstance = {
back: () => window.history.back(),
forward: () => window.history.forward(),
prefetch: async (href) => {
prefetch: async (href: string, options) => {
feedthejim marked this conversation as resolved.
Show resolved Hide resolved
// If prefetch has already been triggered, don't trigger it again.
if (isBot(window.navigator.userAgent)) {
return
Expand All @@ -209,12 +209,12 @@ function Router({
if (isExternalURL(url)) {
return
}

// @ts-ignore startTransition exists
React.startTransition(() => {
dispatch({
type: ACTION_PREFETCH,
url,
kind: options?.kind ?? 'full',
})
})
},
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
wasPrefetched: boolean = false
): boolean {
// The one before last item is the router state tree patch
const [treePatch, subTreeData, head] = flightDataPath.slice(-3)
Expand All @@ -33,7 +33,12 @@ 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)
fillCacheWithNewSubTreeData(
cache,
existingCache,
flightDataPath,
wasPrefetched
)
}

return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ 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 @@ -23,7 +24,7 @@ export async function fetchServerResponse(
url: URL,
flightRouterState: FlightRouterState,
nextUrl: string | null,
prefetch?: true
prefetchKind?: PrefetchKind
): Promise<[FlightData: FlightData, canonicalUrlOverride: URL | undefined]> {
const headers: {
[RSC]: '1'
Expand All @@ -36,8 +37,14 @@ export async function fetchServerResponse(
// Provide the current router state
[NEXT_ROUTER_STATE_TREE]: JSON.stringify(flightRouterState),
}
if (prefetch) {
// Enable prefetch response

/**
* 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 === 'auto') {
headers[NEXT_ROUTER_PREFETCH] = '1'
}

Expand Down
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)
fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath, false)

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

childSegmentMap.set(cacheKey, childCacheNode)
Expand All @@ -90,6 +92,7 @@ export function fillCacheWithNewSubTreeData(
fillCacheWithNewSubTreeData(
childCacheNode,
existingChildCacheNode,
flightDataPath.slice(2)
flightDataPath.slice(2),
wasPrefetched
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { PrefetchCacheEntry } from './router-reducer-types'

const FIVE_MINUTES = 5 * 60 * 1000
const THIRTY_SECONDS = 5 * 1000
feedthejim marked this conversation as resolved.
Show resolved Hide resolved

export enum PrefetchCacheEntryStatus {
reusable = 'reusable',
expired = 'expired',
stale = 'stale',
}

export function getPrefetchEntryCacheStatus({
kind,
prefetchTime,
lastUsedTime,
}: PrefetchCacheEntry): PrefetchCacheEntryStatus {
// if the cache entry was prefetched or read less than 30s ago, then we want to re-use it
if (Date.now() < (lastUsedTime ?? prefetchTime) + THIRTY_SECONDS) {
feedthejim marked this conversation as resolved.
Show resolved Hide resolved
return PrefetchCacheEntryStatus.reusable
}

// if the cache entry was prefetched less than 5 mins ago, then we want to re-use only the loading state
if (kind === 'auto') {
if (Date.now() < prefetchTime + FIVE_MINUTES) {
return PrefetchCacheEntryStatus.stale
}
}

// if the cache entry was prefetched less than 5 mins ago and was a "full" prefetch, then we want to re-use it "full
if (kind === 'full') {
if (Date.now() < prefetchTime + FIVE_MINUTES) {
return PrefetchCacheEntryStatus.reusable
}
}

return PrefetchCacheEntryStatus.expired
}
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)
fillCacheWithNewSubTreeData(cache, existingCache, flightDataPath, false)

// 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 @@ -1005,6 +1005,7 @@ describe('navigateReducer', () => {
const prefetchAction: PrefetchAction = {
type: ACTION_PREFETCH,
url,
kind: 'auto',
}

const state = createInitialRouterState({
Expand Down Expand Up @@ -1086,6 +1087,9 @@ describe('navigateReducer', () => {
'/linking/about',
{
data: record,
kind: 'auto',
lastUsedTime: null,
prefetchTime: expect.any(Number),
treeAtTimeOfPrefetch: [
'',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import type {
} from '../router-reducer-types'
import { handleMutable } from '../handle-mutable'
import { applyFlightData } from '../apply-flight-data'
import {
PrefetchCacheEntryStatus,
getPrefetchEntryCacheStatus,
} from '../get-prefetch-cache-entry-status'

export function handleExternalUrl(
state: ReadonlyReducerState,
Expand Down Expand Up @@ -63,6 +67,17 @@ function generateSegmentsFromPatch(
return segments
}

function prunePrefetchCache(prefetchCache: ReducerState['prefetchCache']) {
for (const [href, prefetchCacheEntry] of prefetchCache) {
if (
getPrefetchEntryCacheStatus(prefetchCacheEntry) ===
PrefetchCacheEntryStatus.expired
) {
prefetchCache.delete(href)
}
}
}

export function navigateReducer(
state: ReadonlyReducerState,
action: NavigateAction
Expand Down Expand Up @@ -90,8 +105,15 @@ export function navigateReducer(
return handleExternalUrl(state, mutable, url.toString(), pendingPush)
}

// we want to prune the prefetch cache on every navigation to avoid it growing too large
prunePrefetchCache(state.prefetchCache)

const prefetchValues = state.prefetchCache.get(createHrefFromUrl(url, false))

if (prefetchValues) {
const prefetchEntryCacheStatus = getPrefetchEntryCacheStatus(prefetchValues)
prefetchValues.lastUsedTime = Date.now()

// The one before last item is the router state tree patch
const { treeAtTimeOfPrefetch, data } = prefetchValues

Expand All @@ -111,7 +133,6 @@ export function navigateReducer(
0,
-3
) as unknown as FlightSegmentPath

// The one before last item is the router state tree patch
const [treePatch] = flightDataPath.slice(-3) as [FlightRouterState]

Expand Down Expand Up @@ -143,7 +164,8 @@ export function navigateReducer(
currentCache,
cache,
flightDataPath,
true
// if the entry is stale, we'll revalidate below the tree and trigger a refetch from the layout router
prefetchEntryCacheStatus !== PrefetchCacheEntryStatus.stale
)

const hardNavigate = shouldHardNavigate(
Expand Down Expand Up @@ -212,14 +234,16 @@ export function navigateReducer(
cache.status = CacheStates.READY
cache.subTreeData = state.cache.subTreeData

const data = fetchServerResponse(url, optimisticTree, state.nextUrl)

// Copy existing cache nodes as far as possible and fill in `data` property with the started data fetch.
// The `data` property is used to suspend in layout-router during render if it hasn't resolved yet by the time it renders.
const res = fillCacheWithDataProperty(
cache,
state.cache,
// TODO-APP: segments.slice(1) strips '', we can get rid of '' altogether.
segments.slice(1),
() => fetchServerResponse(url, optimisticTree, state.nextUrl)
() => data
)

// If optimistic fetch couldn't happen it falls back to the non-optimistic case.
Expand All @@ -232,6 +256,15 @@ export function navigateReducer(
mutable.cache = cache
mutable.canonicalUrl = href

state.prefetchCache.set(createHrefFromUrl(url, false), {
data: Promise.resolve(data),
// this will make sure that the entry will be discarded after 30s
kind: 'temporary',
prefetchTime: Date.now(),
treeAtTimeOfPrefetch: state.tree,
lastUsedTime: Date.now(),
})

return handleMutable(state, mutable)
}
}
Expand All @@ -246,7 +279,18 @@ export function navigateReducer(
}

// Unwrap cache data with `use` to suspend here (in the reducer) until the fetch resolves.
const [flightData, canonicalUrlOverride] = readRecordValue(cache.data!)
const data = readRecordValue(cache.data!)

state.prefetchCache.set(createHrefFromUrl(url, false), {
data: Promise.resolve(data),
// this will make sure that the entry will be discarded after 30s
kind: 'temporary',
prefetchTime: Date.now(),
treeAtTimeOfPrefetch: state.tree,
lastUsedTime: Date.now(),
})

const [flightData, canonicalUrlOverride] = data

// Handle case when navigating to page in `pages` from `app`
if (typeof flightData === 'string') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,12 @@ describe('prefetchReducer', () => {
url,
initialTree,
null,
true
'auto'
)
const action: PrefetchAction = {
type: ACTION_PREFETCH,
url,
kind: 'auto',
}

const newState = await runPromiseThrowChain(() =>
Expand All @@ -149,6 +150,9 @@ describe('prefetchReducer', () => {
'/linking/about',
{
data: record,
kind: 'auto',
lastUsedTime: null,
prefetchTime: expect.any(Number),
treeAtTimeOfPrefetch: [
'',
{
Expand Down Expand Up @@ -273,11 +277,12 @@ describe('prefetchReducer', () => {
url,
initialTree,
null,
true
'auto'
)
const action: PrefetchAction = {
type: ACTION_PREFETCH,
url,
kind: 'auto',
}

await runPromiseThrowChain(() => prefetchReducer(state, action))
Expand All @@ -296,6 +301,9 @@ describe('prefetchReducer', () => {
'/linking/about',
{
data: record,
prefetchTime: expect.any(Number),
kind: 'auto',
lastUsedTime: null,
treeAtTimeOfPrefetch: [
'',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import {
ReadonlyReducerState,
} from '../router-reducer-types'
import { createRecordFromThenable } from '../create-record-from-thenable'
import {
PrefetchCacheEntryStatus,
getPrefetchEntryCacheStatus,
} from '../get-prefetch-cache-entry-status'

export function prefetchReducer(
state: ReadonlyReducerState,
Expand All @@ -18,8 +22,16 @@ export function prefetchReducer(
false
)

const cacheEntry = state.prefetchCache.get(href)
// If the href was already prefetched it is not necessary to prefetch it again
if (state.prefetchCache.has(href)) {
if (
cacheEntry &&
// unless the cache entry was prefetched with 'auto' and the current prefetch is 'full'
(!(cacheEntry.kind === 'auto' && action.kind === 'full') ||
// or the cache entry is expired
getPrefetchEntryCacheStatus(cacheEntry) !==
PrefetchCacheEntryStatus.expired)
) {
return state
}

Expand All @@ -30,7 +42,7 @@ export function prefetchReducer(
// initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
state.tree,
state.nextUrl,
true
action.kind
)
)

Expand All @@ -39,6 +51,9 @@ export function prefetchReducer(
// Create new tree based on the flightSegmentPath and router state patch
treeAtTimeOfPrefetch: state.tree,
data: serverResponse,
kind: action.kind,
prefetchTime: Date.now(),
lastUsedTime: null,
})

return state
Expand Down