From ee88953fc7667e671513fdb36911003195921003 Mon Sep 17 00:00:00 2001 From: agadzik Date: Wed, 3 May 2023 13:22:32 -0400 Subject: [PATCH 1/5] clone request --- packages/react/src/ReactFetch.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ReactFetch.js b/packages/react/src/ReactFetch.js index 7ceea672acd45..9a21439a9729b 100644 --- a/packages/react/src/ReactFetch.js +++ b/packages/react/src/ReactFetch.js @@ -74,7 +74,8 @@ if (enableCache && enableFetchInstrumentation) { url = resource; } else { // Normalize the request. - const request = new Request(resource, options); + // we clone the request as to not disturb the body (in the event it's a ReadableStream) + const request = resource.clone(); if ( (request.method !== 'GET' && request.method !== 'HEAD') || // $FlowFixMe[prop-missing]: keepalive is real From 813c1df868c1f67be09a08aeb40168c691485f0c Mon Sep 17 00:00:00 2001 From: agadzik Date: Wed, 3 May 2023 13:58:51 -0400 Subject: [PATCH 2/5] handle the case where resource is still a string --- packages/react/src/ReactFetch.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ReactFetch.js b/packages/react/src/ReactFetch.js index 9a21439a9729b..d709a107f9b9c 100644 --- a/packages/react/src/ReactFetch.js +++ b/packages/react/src/ReactFetch.js @@ -74,8 +74,9 @@ if (enableCache && enableFetchInstrumentation) { url = resource; } else { // Normalize the request. + // if resource is not a string (its an instance of Request) // we clone the request as to not disturb the body (in the event it's a ReadableStream) - const request = resource.clone(); + const request = typeof resource === 'string' ? new Request(resource, options) : resource.clone(); if ( (request.method !== 'GET' && request.method !== 'HEAD') || // $FlowFixMe[prop-missing]: keepalive is real From bb33b74159cfe7bbf193c2505521111cc2efb1c6 Mon Sep 17 00:00:00 2001 From: agadzik Date: Wed, 3 May 2023 14:18:24 -0400 Subject: [PATCH 3/5] reuse the resource as a Request instead of cloning --- packages/react/src/ReactFetch.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react/src/ReactFetch.js b/packages/react/src/ReactFetch.js index d709a107f9b9c..8c6f14251b911 100644 --- a/packages/react/src/ReactFetch.js +++ b/packages/react/src/ReactFetch.js @@ -75,8 +75,9 @@ if (enableCache && enableFetchInstrumentation) { } else { // Normalize the request. // if resource is not a string (its an instance of Request) - // we clone the request as to not disturb the body (in the event it's a ReadableStream) - const request = typeof resource === 'string' ? new Request(resource, options) : resource.clone(); + // then do not instantiate a new Request but instead + // reuse the request as to not disturb the body in the event it's a ReadableStream. + const request = typeof resource === 'string' ? new Request(resource, options) : resource; if ( (request.method !== 'GET' && request.method !== 'HEAD') || // $FlowFixMe[prop-missing]: keepalive is real From 095ad3a0ed2d29f70459c339fc5799d8ece5093e Mon Sep 17 00:00:00 2001 From: agadzik Date: Wed, 3 May 2023 14:50:08 -0400 Subject: [PATCH 4/5] Handle the case where resource could be a URL --- packages/react/src/ReactFetch.js | 4 ++-- packages/react/src/__tests__/ReactFetch-test.js | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/react/src/ReactFetch.js b/packages/react/src/ReactFetch.js index 8c6f14251b911..8ba707b10813a 100644 --- a/packages/react/src/ReactFetch.js +++ b/packages/react/src/ReactFetch.js @@ -74,10 +74,10 @@ if (enableCache && enableFetchInstrumentation) { url = resource; } else { // Normalize the request. - // if resource is not a string (its an instance of Request) + // if resource is not a string or a URL (its an instance of Request) // then do not instantiate a new Request but instead // reuse the request as to not disturb the body in the event it's a ReadableStream. - const request = typeof resource === 'string' ? new Request(resource, options) : resource; + const request = (typeof resource === 'string' || resource instanceof URL) ? new Request(resource, options) : resource; if ( (request.method !== 'GET' && request.method !== 'HEAD') || // $FlowFixMe[prop-missing]: keepalive is real diff --git a/packages/react/src/__tests__/ReactFetch-test.js b/packages/react/src/__tests__/ReactFetch-test.js index b3f8843b8856e..9558ead7c1bc4 100644 --- a/packages/react/src/__tests__/ReactFetch-test.js +++ b/packages/react/src/__tests__/ReactFetch-test.js @@ -135,6 +135,22 @@ describe('ReactFetch', () => { expect(fetchCount).toBe(1); }); + // @gate enableFetchInstrumentation && enableCache + it('can dedupe fetches using URL and not', async () => { + const url = 'http://example.com/'; + function Component() { + const response = use(fetch(url)); + const text = use(response.text()); + const response2 = use(fetch(new URL(url))); + const text2 = use(response2.text()); + return text + ' ' + text2; + } + expect(await render(Component)).toMatchInlineSnapshot( + `"GET ${url} [] GET ${url} []"`, + ); + expect(fetchCount).toBe(1); + }); + it('can opt-out of deduping fetches inside of render with custom signal', async () => { const controller = new AbortController(); function useCustomHook() { From 70cf10a02bad3590f903c4c6f096cc58a0372e45 Mon Sep 17 00:00:00 2001 From: agadzik Date: Wed, 3 May 2023 14:53:35 -0400 Subject: [PATCH 5/5] fix prettier error --- packages/react/src/ReactFetch.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ReactFetch.js b/packages/react/src/ReactFetch.js index 8ba707b10813a..2a5637efc0d60 100644 --- a/packages/react/src/ReactFetch.js +++ b/packages/react/src/ReactFetch.js @@ -77,7 +77,10 @@ if (enableCache && enableFetchInstrumentation) { // if resource is not a string or a URL (its an instance of Request) // then do not instantiate a new Request but instead // reuse the request as to not disturb the body in the event it's a ReadableStream. - const request = (typeof resource === 'string' || resource instanceof URL) ? new Request(resource, options) : resource; + const request = + typeof resource === 'string' || resource instanceof URL + ? new Request(resource, options) + : resource; if ( (request.method !== 'GET' && request.method !== 'HEAD') || // $FlowFixMe[prop-missing]: keepalive is real