-
Notifications
You must be signed in to change notification settings - Fork 28.5k
fix: fixed dedupe response cloning #73274
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
Merged
+131
−55
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/** | ||
* Clones a response by teeing the body so we can return two independent | ||
* ReadableStreams from it. This avoids the bug in the undici library around | ||
* response cloning. | ||
* | ||
* After cloning, the original response's body will be consumed and closed. | ||
* | ||
* @see https://github.com/vercel/next.js/pull/73274 | ||
* | ||
* @param original - The original response to clone. | ||
* @returns A tuple containing two independent clones of the original response. | ||
*/ | ||
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] | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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')); | ||
|
||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -74,29 +83,38 @@ 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') | ||
|
||
// We're cloning the response using this utility because there exists | ||
// a bug in the undici library around response cloning. See the | ||
// following pull request for more details: | ||
// https://github.com/vercel/next.js/pull/73274 | ||
const [cloned1, cloned2] = cloneResponse(response) | ||
cacheEntries[i][2] = cloned2 | ||
return cloned1 | ||
}) | ||
} | ||
match = originalFetch(resource, options) | ||
cacheEntries.push(cacheKey, match) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it turns out that var args push is slower than non-var-args push in some runtimes so pushing an array appears faster. But the fastest is to call push twice (or after this change, three times) |
||
} | ||
// 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) => { | ||
// We're cloning the response using this utility because there exists | ||
// a bug in the undici library around response cloning. See the | ||
// following pull request for more details: | ||
// https://github.com/vercel/next.js/pull/73274 | ||
const [cloned1, cloned2] = cloneResponse(response) | ||
entry[2] = cloned2 | ||
return cloned1 | ||
}) | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.