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: prefetching tweaks #52949

Merged
41 changes: 41 additions & 0 deletions packages/next/src/client/components/promise-queue.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { PromiseQueue } from './promise-queue'

describe('PromiseQueue', () => {
it('should limit the number of concurrent promises', async () => {
const queue = new PromiseQueue(2)
const results: number[] = []

const promises = Array.from({ length: 5 }, (_, i) =>
queue.enqueue(async () => {
results.push(i)
await new Promise((resolve) => setTimeout(resolve, 100))
return i
})
)

const resolved = await Promise.all(promises)

expect(resolved).toEqual([0, 1, 2, 3, 4])
expect(results).toEqual([0, 1, 2, 3, 4])
})
it('should allow bumping a promise to be next in the queue', async () => {
const queue = new PromiseQueue(2)
const results: number[] = []

const promises = Array.from({ length: 5 }, (_, i) =>
queue.enqueue(async () => {
results.push(i)
await new Promise((resolve) => setTimeout(resolve, 100))
return i
})
)

queue.bump(promises[3])

const resolved = await Promise.all(promises)

// 3 was bumped to be next in the queue but did not cancel the other promises before it
expect(results).toEqual([0, 1, 3, 2, 4])
expect(resolved).toEqual([0, 1, 2, 3, 4])
})
})
66 changes: 66 additions & 0 deletions packages/next/src/client/components/promise-queue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
This is a simple promise queue that allows you to limit the number of concurrent promises
that are running at any given time. It's used to limit the number of concurrent
prefetch requests that are being made to the server but could be used for other
things as well.
*/
export class PromiseQueue {
#maxConcurrency: number
#runningCount: number
#queue: Array<{ promiseFn: Promise<any>; task: () => void }>

constructor(maxConcurrency = 5) {
this.#maxConcurrency = maxConcurrency
this.#runningCount = 0
this.#queue = []
}

enqueue<T>(promiseFn: () => Promise<T>): Promise<T> {
let taskResolve: (value: T | PromiseLike<T>) => void
let taskReject: (reason?: any) => void

const taskPromise = new Promise((resolve, reject) => {
taskResolve = resolve
taskReject = reject
}) as Promise<T>

const task = async () => {
try {
this.#runningCount++
const result = await promiseFn()
taskResolve(result)
} catch (error) {
taskReject(error)
} finally {
this.#runningCount--
this.#processNext()
}
}

const enqueueResult = { promiseFn: taskPromise, task }
// wonder if we should take a LIFO approach here
this.#queue.push(enqueueResult)
this.#processNext()

return taskPromise
}

bump(promiseFn: Promise<any>) {
const index = this.#queue.findIndex((item) => item.promiseFn === promiseFn)

if (index > -1) {
const bumpedItem = this.#queue.splice(index, 1)[0]
this.#queue.unshift(bumpedItem)
this.#processNext(true)
}
}

#processNext(forced = false) {
if (
(this.#runningCount < this.#maxConcurrency || forced) &&
this.#queue.length > 0
) {
this.#queue.shift()?.task()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
getPrefetchEntryCacheStatus,
} from '../get-prefetch-cache-entry-status'
import { prunePrefetchCache } from './prune-prefetch-cache'
import { prefetchQueue } from './prefetch-reducer'

export function handleExternalUrl(
state: ReadonlyReducerState,
Expand Down Expand Up @@ -245,6 +246,8 @@ export function navigateReducer(
// The one before last item is the router state tree patch
const { treeAtTimeOfPrefetch, data } = prefetchValues

prefetchQueue.bump(data!)

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import {
import { createRecordFromThenable } from '../create-record-from-thenable'
import { prunePrefetchCache } from './prune-prefetch-cache'
import { NEXT_RSC_UNION_QUERY } from '../../app-router-headers'
import { PromiseQueue } from '../../promise-queue'

export const prefetchQueue = new PromiseQueue(5)

export function prefetchReducer(
state: ReadonlyReducerState,
Expand Down Expand Up @@ -55,13 +58,15 @@ export function prefetchReducer(

// fetchServerResponse is intentionally not awaited so that it can be unwrapped in the navigate-reducer
const serverResponse = createRecordFromThenable(
fetchServerResponse(
url,
// 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,
state.buildId,
action.kind
prefetchQueue.enqueue(() =>
fetchServerResponse(
url,
// 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,
state.buildId,
action.kind
)
)
)

Expand Down
98 changes: 60 additions & 38 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@ function findDynamicParamFromRouterState(
return null
}

function hasLoadingComponentInTree(tree: LoaderTree): boolean {
const [, parallelRoutes, { loading }] = tree

if (loading) {
return true
}

return Object.values(parallelRoutes).some((parallelRoute) =>
hasLoadingComponentInTree(parallelRoute)
) as boolean
}

export async function renderToHTMLOrFlight(
req: IncomingMessage,
res: ServerResponse,
Expand Down Expand Up @@ -810,12 +822,17 @@ export async function renderToHTMLOrFlight(
: [actualSegment, parallelRouteKey]

const parallelRoute = parallelRoutes[parallelRouteKey]

const childSegment = parallelRoute[0]
const childSegmentParam = getDynamicParamFromSegment(childSegment)

// if we're prefetching and that there's a Loading component, we bail out
// otherwise we keep rendering for the prefetch
if (isPrefetch && Loading) {
// otherwise we keep rendering for the prefetch.
// We also want to bail out if there's no Loading component in the tree.
if (
isPrefetch &&
(Loading || !hasLoadingComponentInTree(parallelRoute))
) {
const childProp: ChildProp = {
// Null indicates the tree is not fully rendered
current: null,
Expand Down Expand Up @@ -1105,42 +1122,47 @@ export async function renderToHTMLOrFlight(
getDynamicParamFromSegment,
query
),
// Create component tree using the slice of the loaderTree
// @ts-expect-error TODO-APP: fix async component type
React.createElement(async () => {
const { Component } = await createComponentTree(
// This ensures flightRouterPath is valid and filters down the tree
{
createSegmentPath,
loaderTree: loaderTreeToFilter,
parentParams: currentParams,
firstItem: isFirst,
injectedCSS,
injectedFontPreloadTags,
// This is intentionally not "rootLayoutIncludedAtThisLevelOrAbove" as createComponentTree starts at the current level and does a check for "rootLayoutAtThisLevel" too.
rootLayoutIncluded,
asNotFound,
}
)

return <Component />
}),
(() => {
const { layoutOrPagePath } = parseLoaderTree(loaderTreeToFilter)

const styles = getLayerAssets({
layoutOrPagePath,
injectedCSS: new Set(injectedCSS),
injectedFontPreloadTags: new Set(injectedFontPreloadTags),
})

return (
<>
{styles}
{rscPayloadHead}
</>
)
})(),
isPrefetch && !Boolean(components.loading)
? null
: // Create component tree using the slice of the loaderTree
// @ts-expect-error TODO-APP: fix async component type
React.createElement(async () => {
const { Component } = await createComponentTree(
// This ensures flightRouterPath is valid and filters down the tree
{
createSegmentPath,
loaderTree: loaderTreeToFilter,
parentParams: currentParams,
firstItem: isFirst,
injectedCSS,
injectedFontPreloadTags,
// This is intentionally not "rootLayoutIncludedAtThisLevelOrAbove" as createComponentTree starts at the current level and does a check for "rootLayoutAtThisLevel" too.
rootLayoutIncluded,
asNotFound,
}
)

return <Component />
}),
isPrefetch && !Boolean(components.loading)
? null
: (() => {
const { layoutOrPagePath } =
parseLoaderTree(loaderTreeToFilter)

const styles = getLayerAssets({
layoutOrPagePath,
injectedCSS: new Set(injectedCSS),
injectedFontPreloadTags: new Set(injectedFontPreloadTags),
})

return (
<>
{styles}
{rscPayloadHead}
</>
)
})(),
],
]
}
Expand Down
10 changes: 9 additions & 1 deletion packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,15 @@ export default abstract class Server<ServerOptions extends Options = Options> {
if (isAppPath) {
res.setHeader('vary', RSC_VARY_HEADER)

if (!isPreviewMode && isSSG && req.headers[RSC.toLowerCase()]) {
// We don't clear RSC headers in development since SSG doesn't apply
// These headers are cleared for SSG as we need to always generate the
// full RSC response for ISR
if (
!this.renderOpts.dev &&
!isPreviewMode &&
isSSG &&
req.headers[RSC.toLowerCase()]
) {
if (!this.minimalMode) {
isDataReq = true
}
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ createNextDescribe(
}, 'success')
})

skipDeploy('should handle revalidateTag + redirect', async () => {
// TODO: investigate flakey behavior
it.skip('should handle revalidateTag + redirect', async () => {
const browser = await next.browser('/revalidate')
const randomNumber = await browser.elementByCss('#random-number').text()
const justPutIt = await browser.elementByCss('#justputit').text()
Expand Down