Skip to content

Commit

Permalink
fix: fixed dedupe response cloning
Browse files Browse the repository at this point in the history
  • Loading branch information
wyattjoh committed Dec 4, 2024
1 parent 8756f7a commit 5456ca2
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 54 deletions.
31 changes: 31 additions & 0 deletions packages/next/src/server/lib/clone-response.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
export function cloneResponse(original: Response): [Response, Response] {
// If the response has no body, then we can just return the original response
// twice because it's immutable.
if (!original.body) {
return [original, original]
}

const [body1, body2] = original.body.tee()

const cloned1 = new Response(body1, {
status: original.status,
statusText: original.statusText,
headers: original.headers,
})

Object.defineProperty(cloned1, 'url', {
value: original.url,
})

const cloned2 = new Response(body2, {
status: original.status,
statusText: original.statusText,
headers: original.headers,
})

Object.defineProperty(cloned2, 'url', {
value: original.url,
})

return [cloned1, cloned2]
}
62 changes: 36 additions & 26 deletions packages/next/src/server/lib/dedupe-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
* Based on https://github.com/facebook/react/blob/d4e78c42a94be027b4dc7ed2659a5fddfbf9bd4e/packages/react/src/ReactFetch.js
*/
import * as React from 'react'
import { cloneResponse } from './clone-response'
import { InvariantError } from '../../shared/lib/invariant-error'

const simpleCacheKey = '["GET",[],null,"follow",null,null,null,null]' // generateCacheKey(new Request('https://blank'));

Expand All @@ -24,14 +26,22 @@ function generateCacheKey(request: Request): string {
])
}

type CacheEntry = [
key: string,
promise: Promise<Response>,
response: Response | null,
]

export function createDedupeFetch(originalFetch: typeof fetch) {
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- url is the cache key
const getCacheEntries = React.cache((url: string): Array<any> => [])
const getCacheEntries = React.cache(
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- url is the cache key
(url: string): CacheEntry[] => []
)

return function dedupeFetch(
resource: URL | RequestInfo,
options?: RequestInit
) {
): Promise<Response> {
if (options && options.signal) {
// If we're passed a signal, then we assume that
// someone else controls the lifetime of this object and opts out of
Expand Down Expand Up @@ -60,7 +70,6 @@ export function createDedupeFetch(originalFetch: typeof fetch) {
: resource
if (
(request.method !== 'GET' && request.method !== 'HEAD') ||
// $FlowFixMe[prop-missing]: keepalive is real
request.keepalive
) {
// We currently don't dedupe requests that might have side-effects. Those
Expand All @@ -74,29 +83,30 @@ export function createDedupeFetch(originalFetch: typeof fetch) {
}

const cacheEntries = getCacheEntries(url)
let match
if (cacheEntries.length === 0) {
// We pass the original arguments here in case normalizing the Request
// doesn't include all the options in this environment.
match = originalFetch(resource, options)
cacheEntries.push(cacheKey, match)
} else {
// We use an array as the inner data structure since it's lighter and
// we typically only expect to see one or two entries here.
for (let i = 0, l = cacheEntries.length; i < l; i += 2) {
const key = cacheEntries[i]
const value = cacheEntries[i + 1]
if (key === cacheKey) {
match = value
// I would've preferred a labelled break but lint says no.
return match.then((response: Response) => response.clone())
}
for (let i = 0, j = cacheEntries.length; i < j; i += 1) {
const [key, promise] = cacheEntries[i]
if (key === cacheKey) {
return promise.then(() => {
const response = cacheEntries[i][2]
if (!response) throw new InvariantError('No cached response')

const [cloned1, cloned2] = cloneResponse(response)
cacheEntries[i][2] = cloned2
return cloned1
})
}
match = originalFetch(resource, options)
cacheEntries.push(cacheKey, match)
}
// We clone the response so that each time you call this you get a new read
// of the body so that it can be read multiple times.
return match.then((response) => response.clone())

// We pass the original arguments here in case normalizing the Request
// doesn't include all the options in this environment.
const promise = originalFetch(resource, options)
const entry: CacheEntry = [cacheKey, promise, null]
cacheEntries.push(entry)

return promise.then((response) => {
const [cloned1, cloned2] = cloneResponse(response)
entry[2] = cloned2
return cloned1
})
}
}
65 changes: 37 additions & 28 deletions packages/next/src/server/lib/patch-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
type CachedFetchData,
} from '../response-cache'
import { waitAtLeastOneReactRenderTask } from '../../lib/scheduler'
import { cloneResponse } from './clone-response'

const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'

Expand Down Expand Up @@ -676,20 +677,21 @@ export function createPatchedFetcher(
statusText: res.statusText,
})
} else {
const [cloned1, cloned2] = cloneResponse(res)

// We are dynamically rendering including dev mode. We want to return
// the response to the caller as soon as possible because it might stream
// over a very long time.
res
.clone()
cloned1
.arrayBuffer()
.then(async (arrayBuffer) => {
const bodyBuffer = Buffer.from(arrayBuffer)

const fetchedData = {
headers: Object.fromEntries(res.headers.entries()),
headers: Object.fromEntries(cloned1.headers.entries()),
body: bodyBuffer.toString('base64'),
status: res.status,
url: res.url,
status: cloned1.status,
url: cloned1.url,
}

requestStore?.serverComponentsHmrCache?.set(
Expand Down Expand Up @@ -720,7 +722,7 @@ export function createPatchedFetcher(
)
.finally(handleUnlock)

return res
return cloned2
}
}

Expand Down Expand Up @@ -788,14 +790,23 @@ export function createPatchedFetcher(
if (entry.isStale) {
workStore.pendingRevalidates ??= {}
if (!workStore.pendingRevalidates[cacheKey]) {
workStore.pendingRevalidates[cacheKey] = doOriginalFetch(
true
)
.catch(console.error)
const pendingRevalidate = doOriginalFetch(true)
.then(async (response) => ({
body: await response.arrayBuffer(),
headers: response.headers,
status: response.status,
statusText: response.statusText,
}))
.finally(() => {
workStore.pendingRevalidates ??= {}
delete workStore.pendingRevalidates[cacheKey || '']
})

// Attach the empty catch here so we don't get a "unhandled
// promise rejection" warning.
pendingRevalidate.catch(console.error)

workStore.pendingRevalidates[cacheKey] = pendingRevalidate
}
}

Expand Down Expand Up @@ -895,7 +906,7 @@ export function createPatchedFetcher(
if (cacheKey && isForegroundRevalidate) {
const pendingRevalidateKey = cacheKey
workStore.pendingRevalidates ??= {}
const pendingRevalidate =
let pendingRevalidate =
workStore.pendingRevalidates[pendingRevalidateKey]

if (pendingRevalidate) {
Expand All @@ -920,21 +931,19 @@ export function createPatchedFetcher(
// available we construct manually cloned Response objects with the
// body as an ArrayBuffer. This will be resolvable in a microtask
// making it compatible with dynamicIO.
const pendingResponse = doOriginalFetch(true, cacheReasonOverride)

const nextRevalidate = pendingResponse
.then(async (response) => {
// Clone the response here. It'll run first because we attached
// the resolve before we returned below. We have to clone it
// because the original response is going to be consumed by
// at a later point in time.
const clonedResponse = response.clone()

const pendingResponse = doOriginalFetch(
true,
cacheReasonOverride
).then(cloneResponse)

pendingRevalidate = pendingResponse
.then(async (responses) => {
const response = responses[0]
return {
body: await clonedResponse.arrayBuffer(),
headers: clonedResponse.headers,
status: clonedResponse.status,
statusText: clonedResponse.statusText,
body: await response.arrayBuffer(),
headers: response.headers,
status: response.status,
statusText: response.statusText,
}
})
.finally(() => {
Expand All @@ -949,11 +958,11 @@ export function createPatchedFetcher(

// Attach the empty catch here so we don't get a "unhandled promise
// rejection" warning
nextRevalidate.catch(() => {})
pendingRevalidate.catch(() => {})

workStore.pendingRevalidates[pendingRevalidateKey] = nextRevalidate
workStore.pendingRevalidates[pendingRevalidateKey] = pendingRevalidate

return pendingResponse
return pendingResponse.then((responses) => responses[1])
} else {
return doOriginalFetch(false, cacheReasonOverride)
}
Expand Down

0 comments on commit 5456ca2

Please sign in to comment.