Remove automatic fetch `cache` instrumentation (#28896)
This removes the automatic patching of the global `fetch` function in Server Components environments to dedupe requests using `React.cache`, a behavior that some RSC framework maintainers have objected to. We may revisit this decision in the future, but for now it's not worth the controversy. Frameworks that have already shipped this behavior, like Next.js, can reimplement it in userspace. I considered keeping the implementation in the codebase and disabling it by setting `enableFetchInstrumentation` to `false` everywhere, but since that also disables the tests, it doesn't seem worth it because without test coverage the behavior is likely to drift regardless. We can just revert this PR later if desired.
Andrew Clark committed
Apr 23, 2024 at 14:14 UTC
a94838df1c598a3993316ff453c84f3688537a97
12 files changed
-459
packages/react/src/ReactFetch.js
deleted
-141
@@ -1,141 +0,0 @@
1
-/**
2
- * Copyright (c) Meta Platforms, Inc. and its affiliates.
3
- *
4
- * This source code is licensed under the MIT license found in the
5
- * LICENSE file in the root directory of this source tree.
6
- *
7
- * @flow
8
- */
9
-
10
-import {
11
- enableCache,
12
- enableFetchInstrumentation,
13
-} from 'shared/ReactFeatureFlags';
14
-
15
-import ReactSharedInternals from 'shared/ReactSharedInternals';
16
-
17
-function createFetchCache(): Map<string, Array<any>> {
18
- return new Map();
19
-}
20
-
21
-const simpleCacheKey = '["GET",[],null,"follow",null,null,null,null]'; // generateCacheKey(new Request('https://blank'));
22
-
23
-function generateCacheKey(request: Request): string {
24
- // We pick the fields that goes into the key used to dedupe requests.
25
- // We don't include the `cache` field, because we end up using whatever
26
- // caching resulted from the first request.
27
- // Notably we currently don't consider non-standard (or future) options.
28
- // This might not be safe. TODO: warn for non-standard extensions differing.
29
- // IF YOU CHANGE THIS UPDATE THE simpleCacheKey ABOVE.
30
- return JSON.stringify([
31
- request.method,
32
- Array.from(request.headers.entries()),
33
- request.mode,
34
- request.redirect,
35
- request.credentials,
36
- request.referrer,
37
- request.referrerPolicy,
38
- request.integrity,
39
- ]);
40
-}
41
-
42
-if (enableCache && enableFetchInstrumentation) {
43
- if (typeof fetch === 'function') {
44
- const originalFetch = fetch;
45
- const cachedFetch = function fetch(
46
- resource: URL | RequestInfo,
47
- options?: RequestOptions,
48
- ) {
49
- const dispatcher = ReactSharedInternals.C;
50
- if (!dispatcher) {
51
- // We're outside a cached scope.
52
- return originalFetch(resource, options);
53
- }
54
- if (options && options.signal) {
55
- // If we're passed a signal, then we assume that
56
- // someone else controls the lifetime of this object and opts out of
57
- // caching. It's effectively the opt-out mechanism.
58
- // Ideally we should be able to check this on the Request but
59
- // it always gets initialized with its own signal so we don't
60
- // know if it's supposed to override - unless we also override the
61
- // Request constructor.
62
- return originalFetch(resource, options);
63
- }
64
- // Normalize the Request
65
- let url: string;
66
- let cacheKey: string;
67
- if (typeof resource === 'string' && !options) {
68
- // Fast path.
69
- cacheKey = simpleCacheKey;
70
- url = resource;
71
- } else {
72
- // Normalize the request.
73
- // if resource is not a string or a URL (its an instance of Request)
74
- // then do not instantiate a new Request but instead
75
- // reuse the request as to not disturb the body in the event it's a ReadableStream.
76
- const request =
77
- typeof resource === 'string' || resource instanceof URL
78
- ? new Request(resource, options)
79
- : resource;
80
- if (
81
- (request.method !== 'GET' && request.method !== 'HEAD') ||
82
- // $FlowFixMe[prop-missing]: keepalive is real
83
- request.keepalive
84
- ) {
85
- // We currently don't dedupe requests that might have side-effects. Those
86
- // have to be explicitly cached. We assume that the request doesn't have a
87
- // body if it's GET or HEAD.
88
- // keepalive gets treated the same as if you passed a custom cache signal.
89
- return originalFetch(resource, options);
90
- }
91
- cacheKey = generateCacheKey(request);
92
- url = request.url;
93
- }
94
- const cache = dispatcher.getCacheForType(createFetchCache);
95
- const cacheEntries = cache.get(url);
96
- let match;
97
- if (cacheEntries === undefined) {
98
- // We pass the original arguments here in case normalizing the Request
99
- // doesn't include all the options in this environment.
100
- match = originalFetch(resource, options);
101
- cache.set(url, [cacheKey, match]);
102
- } else {
103
- // We use an array as the inner data structure since it's lighter and
104
- // we typically only expect to see one or two entries here.
105
- for (let i = 0, l = cacheEntries.length; i < l; i += 2) {
106
- const key = cacheEntries[i];
107
- const value = cacheEntries[i + 1];
108
- if (key === cacheKey) {
109
- match = value;
110
- // I would've preferred a labelled break but lint says no.
111
- return match.then(response => response.clone());
112
- }
113
- }
114
- match = originalFetch(resource, options);
115
- cacheEntries.push(cacheKey, match);
116
- }
117
- // We clone the response so that each time you call this you get a new read
118
- // of the body so that it can be read multiple times.
119
- return match.then(response => response.clone());
120
- };
121
- // We don't expect to see any extra properties on fetch but if there are any,
122
- // copy them over. Useful for extended fetch environments or mocks.
123
- Object.assign(cachedFetch, originalFetch);
124
- try {
125
- // eslint-disable-next-line no-native-reassign
126
- fetch = cachedFetch;
127
- } catch (error1) {
128
- try {
129
- // In case assigning it globally fails, try globalThis instead just in case it exists.
130
- globalThis.fetch = cachedFetch;
131
- } catch (error2) {
132
- // Log even in production just to make sure this is seen if only prod is frozen.
133
- // eslint-disable-next-line react-internal/no-production-logging
134
- console.warn(
135
- 'React was unable to patch the fetch() function in this environment. ' +
136
- 'Suspensey APIs might not work correctly as a result.',
137
- );
138
- }
139
- }
140
- }
141
-}
packages/react/src/ReactServer.experimental.js
-3
@@ -7,9 +7,6 @@
7
* @flow
8
*/
9
10
-// Patch fetch
11
-import './ReactFetch';
12
-
10
export {default as __SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE} from './ReactSharedInternalsServer';
11
12
import {forEach, map, count, toArray, only} from './ReactChildren';
packages/react/src/ReactServer.js
-3
@@ -7,9 +7,6 @@
7
* @flow
8
*/
9
10
-// Patch fetch
11
-import './ReactFetch';
12
-
10
export {default as __SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE} from './ReactSharedInternalsServer';
11
12
import {forEach, map, count, toArray, only} from './ReactChildren';
packages/react/src/__tests__/ReactFetch-test.js
deleted
-215
@@ -1,215 +0,0 @@
1
-/**
2
- * Copyright (c) Meta Platforms, Inc. and its affiliates.
3
- *
4
- * This source code is licensed under the MIT license found in the
5
- * LICENSE file in the root directory of this source tree.
6
- *
7
- * @emails react-core
8
- */
9
-
10
-'use strict';
11
-
12
-// Polyfills for test environment
13
-global.ReadableStream =
14
- require('web-streams-polyfill/ponyfill/es6').ReadableStream;
15
-global.TextEncoder = require('util').TextEncoder;
16
-global.TextDecoder = require('util').TextDecoder;
17
-global.Headers = require('node-fetch').Headers;
18
-global.Request = require('node-fetch').Request;
19
-global.Response = require('node-fetch').Response;
20
-
21
-let fetchCount = 0;
22
-async function fetchMock(resource, options) {
23
- fetchCount++;
24
- const request = new Request(resource, options);
25
- return new Response(
26
- request.method +
27
- ' ' +
28
- request.url +
29
- ' ' +
30
- JSON.stringify(Array.from(request.headers.entries())),
31
- );
32
-}
33
-
34
-let React;
35
-let ReactServer;
36
-let ReactServerDOMServer;
37
-let ReactServerDOMClient;
38
-let use;
39
-let cache;
40
-
41
-describe('ReactFetch', () => {
42
- beforeEach(() => {
43
- jest.resetModules();
44
- fetchCount = 0;
45
- global.fetch = fetchMock;
46
-
47
- jest.mock('react', () => require('react/react.react-server'));
48
- jest.mock('react-server-dom-webpack/server', () =>
49
- require('react-server-dom-webpack/server.browser'),
50
- );
51
- require('react-server-dom-webpack/src/__tests__/utils/WebpackMock');
52
- ReactServerDOMServer = require('react-server-dom-webpack/server');
53
- ReactServer = require('react');
54
-
55
- jest.resetModules();
56
- __unmockReact();
57
- jest.unmock('react-server-dom-webpack/server');
58
- ReactServerDOMClient = require('react-server-dom-webpack/client');
59
- React = require('react');
60
- use = ReactServer.use;
61
- cache = ReactServer.cache;
62
- });
63
-
64
- function render(Component) {
65
- const stream = ReactServerDOMServer.renderToReadableStream(<Component />);
66
- return ReactServerDOMClient.createFromReadableStream(stream);
67
- }
68
-
69
- it('can fetch duplicates outside of render', async () => {
70
- let response = await fetch('world');
71
- let text = await response.text();
72
- expect(text).toMatchInlineSnapshot(`"GET world []"`);
73
- response = await fetch('world');
74
- text = await response.text();
75
- expect(text).toMatchInlineSnapshot(`"GET world []"`);
76
- expect(fetchCount).toBe(2);
77
- });
78
-
79
- // @gate enableFetchInstrumentation && enableCache
80
- it('can dedupe fetches inside of render', async () => {
81
- function Component() {
82
- const response = use(fetch('world'));
83
- const text = use(response.text());
84
- return text;
85
- }
86
- const promise = render(Component);
87
- expect(await promise).toMatchInlineSnapshot(`"GET world []"`);
88
- expect(promise._debugInfo).toEqual(
89
- __DEV__ ? [{name: 'Component', env: 'Server', owner: null}] : undefined,
90
- );
91
- expect(fetchCount).toBe(1);
92
- });
93
-
94
- // @gate enableFetchInstrumentation && enableCache
95
- it('can dedupe fetches in micro tasks', async () => {
96
- async function getData() {
97
- const r1 = await fetch('hello');
98
- const t1 = await r1.text();
99
- const r2 = await fetch('world');
100
- const t2 = await r2.text();
101
- return t1 + ' ' + t2;
102
- }
103
- function Component() {
104
- return use(getData());
105
- }
106
- expect(await render(Component)).toMatchInlineSnapshot(
107
- `"GET hello [] GET world []"`,
108
- );
109
- expect(fetchCount).toBe(2);
110
- });
111
-
112
- // @gate enableFetchInstrumentation && enableCache
113
- it('can dedupe cache in micro tasks', async () => {
114
- const cached = cache(async () => {
115
- fetchCount++;
116
- return 'world';
117
- });
118
- async function getData() {
119
- const r1 = await fetch('hello');
120
- const t1 = await r1.text();
121
- const t2 = await cached();
122
- return t1 + ' ' + t2;
123
- }
124
- function Component() {
125
- return use(getData());
126
- }
127
- expect(await render(Component)).toMatchInlineSnapshot(
128
- `"GET hello [] world"`,
129
- );
130
- expect(fetchCount).toBe(2);
131
- });
132
-
133
- // @gate enableFetchInstrumentation && enableCache
134
- it('can dedupe fetches using Request and not', async () => {
135
- function Component() {
136
- const response = use(fetch('world'));
137
- const text = use(response.text());
138
- const sameRequest = new Request('world', {method: 'get'});
139
- const response2 = use(fetch(sameRequest));
140
- const text2 = use(response2.text());
141
- return text + ' ' + text2;
142
- }
143
- expect(await render(Component)).toMatchInlineSnapshot(
144
- `"GET world [] GET world []"`,
145
- );
146
- expect(fetchCount).toBe(1);
147
- });
148
-
149
- // @gate enableFetchInstrumentation && enableCache
150
- it('can dedupe fetches using URL and not', async () => {
151
- const url = 'http://example.com/';
152
- function Component() {
153
- const response = use(fetch(url));
154
- const text = use(response.text());
155
- const response2 = use(fetch(new URL(url)));
156
- const text2 = use(response2.text());
157
- return text + ' ' + text2;
158
- }
159
- expect(await render(Component)).toMatchInlineSnapshot(
160
- `"GET ${url} [] GET ${url} []"`,
161
- );
162
- expect(fetchCount).toBe(1);
163
- });
164
-
165
- it('can opt-out of deduping fetches inside of render with custom signal', async () => {
166
- const controller = new AbortController();
167
- function useCustomHook() {
168
- return use(
169
- fetch('world', {signal: controller.signal}).then(response =>
170
- response.text(),
171
- ),
172
- );
173
- }
174
- function Component() {
175
- return useCustomHook() + ' ' + useCustomHook();
176
- }
177
- expect(await render(Component)).toMatchInlineSnapshot(
178
- `"GET world [] GET world []"`,
179
- );
180
- expect(fetchCount).not.toBe(1);
181
- });
182
-
183
- it('opts out of deduping for POST requests', async () => {
184
- function useCustomHook() {
185
- return use(
186
- fetch('world', {method: 'POST'}).then(response => response.text()),
187
- );
188
- }
189
- function Component() {
190
- return useCustomHook() + ' ' + useCustomHook();
191
- }
192
- expect(await render(Component)).toMatchInlineSnapshot(
193
- `"POST world [] POST world []"`,
194
- );
195
- expect(fetchCount).not.toBe(1);
196
- });
197
-
198
- // @gate enableFetchInstrumentation && enableCache
199
- it('can dedupe fetches using same headers but not different', async () => {
200
- function Component() {
201
- const response = use(fetch('world', {headers: {a: 'A'}}));
202
- const text = use(response.text());
203
- const sameRequest = new Request('world', {
204
- headers: new Headers({b: 'B'}),
205
- });
206
- const response2 = use(fetch(sameRequest));
207
- const text2 = use(response2.text());
208
- return text + ' ' + text2;
209
- }
210
- expect(await render(Component)).toMatchInlineSnapshot(
211
- `"GET world [["a","A"]] GET world [["b","B"]]"`,
212
- );
213
- expect(fetchCount).toBe(2);
214
- });
215
-});
packages/react/src/__tests__/ReactFetchEdge-test.js
deleted
-90
@@ -1,90 +0,0 @@
1
-/**
2
- * Copyright (c) Meta Platforms, Inc. and its affiliates.
3
- *
4
- * This source code is licensed under the MIT license found in the
5
- * LICENSE file in the root directory of this source tree.
6
- *
7
- * @emails react-core
8
- */
9
-
10
-'use strict';
11
-
12
-// Polyfills for test environment
13
-global.ReadableStream =
14
- require('web-streams-polyfill/ponyfill/es6').ReadableStream;
15
-global.TextEncoder = require('util').TextEncoder;
16
-global.TextDecoder = require('util').TextDecoder;
17
-global.Headers = require('node-fetch').Headers;
18
-global.Request = require('node-fetch').Request;
19
-global.Response = require('node-fetch').Response;
20
-// Patch for Edge environments for global scope
21
-global.AsyncLocalStorage = require('async_hooks').AsyncLocalStorage;
22
-
23
-// Don't wait before processing work on the server.
24
-// TODO: we can replace this with FlightServer.act().
25
-global.setTimeout = cb => cb();
26
-
27
-let fetchCount = 0;
28
-async function fetchMock(resource, options) {
29
- fetchCount++;
30
- const request = new Request(resource, options);
31
- return new Response(
32
- request.method +
33
- ' ' +
34
- request.url +
35
- ' ' +
36
- JSON.stringify(Array.from(request.headers.entries())),
37
- );
38
-}
39
-
40
-let React;
41
-let ReactServerDOMServer;
42
-let ReactServerDOMClient;
43
-let use;
44
-
45
-describe('ReactFetch', () => {
46
- beforeEach(() => {
47
- jest.resetModules();
48
- fetchCount = 0;
49
- global.fetch = fetchMock;
50
-
51
- jest.mock('react', () => require('react/react.react-server'));
52
- jest.mock('react-server-dom-webpack/server', () =>
53
- require('react-server-dom-webpack/server.edge'),
54
- );
55
- require('react-server-dom-webpack/src/__tests__/utils/WebpackMock');
56
-
57
- React = require('react');
58
- ReactServerDOMServer = require('react-server-dom-webpack/server');
59
-
60
- jest.resetModules();
61
- __unmockReact();
62
- jest.unmock('react-server-dom-webpack/server');
63
- ReactServerDOMClient = require('react-server-dom-webpack/client');
64
- use = React.use;
65
- });
66
-
67
- async function render(Component) {
68
- const stream = ReactServerDOMServer.renderToReadableStream(<Component />);
69
- return ReactServerDOMClient.createFromReadableStream(stream);
70
- }
71
-
72
- // @gate enableFetchInstrumentation && enableCache
73
- it('can dedupe fetches separately in interleaved renders', async () => {
74
- async function getData() {
75
- const r1 = await fetch('hi');
76
- const t1 = await r1.text();
77
- const r2 = await fetch('hi');
78
- const t2 = await r2.text();
79
- return t1 + ' ' + t2;
80
- }
81
- function Component() {
82
- return use(getData());
83
- }
84
- const render1 = render(Component);
85
- const render2 = render(Component);
86
- expect(await render1).toMatchInlineSnapshot(`"GET hi [] GET hi []"`);
87
- expect(await render2).toMatchInlineSnapshot(`"GET hi [] GET hi []"`);
88
- expect(fetchCount).toBe(2);
89
- });
90
-});
packages/shared/ReactFeatureFlags.js
-1
@@ -78,7 +78,6 @@ export const enableLegacyFBSupport = false;
78
79
export const enableCache = true;
80
export const enableLegacyCache = __EXPERIMENTAL__;
81
-export const enableFetchInstrumentation = true;
81
82
export const enableBinaryFlight = __EXPERIMENTAL__;
83
export const enableFlightReadableStream = __EXPERIMENTAL__;
packages/shared/forks/ReactFeatureFlags.native-fb.js
-1
@@ -43,7 +43,6 @@ export const enableProfilerNestedUpdatePhase = __PROFILE__;
43
export const enableUpdaterTracking = __PROFILE__;
44
export const enableCache = true;
45
export const enableLegacyCache = false;
46
-export const enableFetchInstrumentation = false;
46
export const enableBinaryFlight = true;
47
export const enableFlightReadableStream = true;
48
export const enableAsyncIterableChildren = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
-1
@@ -73,7 +73,6 @@ export const enableDebugTracing = false;
73
export const enableAsyncDebugInfo = false;
74
export const enableSchedulingProfiler = __PROFILE__;
75
export const enableLegacyCache = false;
76
-export const enableFetchInstrumentation = false;
76
export const enablePostpone = false;
77
export const disableCommentsAsDOMContainers = true;
78
export const disableInputAttributeSyncing = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
-1
@@ -20,7 +20,6 @@ export const enableProfilerNestedUpdatePhase = __PROFILE__;
20
export const enableUpdaterTracking = false;
21
export const enableCache = true;
22
export const enableLegacyCache = __EXPERIMENTAL__;
23
-export const enableFetchInstrumentation = true;
23
export const enableBinaryFlight = true;
24
export const enableFlightReadableStream = true;
25
export const enableAsyncIterableChildren = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js
-1
@@ -20,7 +20,6 @@ export const enableProfilerNestedUpdatePhase = __PROFILE__;
20
export const enableUpdaterTracking = false;
21
export const enableCache = true;
22
export const enableLegacyCache = false;
23
-export const enableFetchInstrumentation = false;
23
export const enableBinaryFlight = true;
24
export const enableFlightReadableStream = true;
25
export const enableAsyncIterableChildren = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
-1
@@ -20,7 +20,6 @@ export const enableProfilerNestedUpdatePhase = __PROFILE__;
20
export const enableUpdaterTracking = false;
21
export const enableCache = true;
22
export const enableLegacyCache = true;
23
-export const enableFetchInstrumentation = false;
23
export const enableBinaryFlight = true;
24
export const enableFlightReadableStream = true;
25
export const enableAsyncIterableChildren = false;
packages/shared/forks/ReactFeatureFlags.www.js
-1
@@ -69,7 +69,6 @@ export const renameElementSymbol = false;
69
70
export const enableCache = true;
71
export const enableLegacyCache = true;
72
-export const enableFetchInstrumentation = false;
72
73
export const enableBinaryFlight = false;
74
export const enableFlightReadableStream = false;