From bccbbb5a57c9519aedc1e01f130afae297791a47 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Fri, 27 Jan 2023 22:52:39 +0100 Subject: [PATCH 1/3] fix hydration during streaming ssr --- _internal/utils/helper.ts | 20 ++++++- core/use-swr.ts | 46 +++++++++------- test/use-swr-streaming-ssr.test.tsx | 81 +++++++++++++++++++++++++++++ test/utils.tsx | 35 +++++++++++++ 4 files changed, 161 insertions(+), 21 deletions(-) create mode 100644 test/use-swr-streaming-ssr.test.tsx diff --git a/_internal/utils/helper.ts b/_internal/utils/helper.ts index c1bd2a944..8d9b8401c 100644 --- a/_internal/utils/helper.ts +++ b/_internal/utils/helper.ts @@ -2,6 +2,7 @@ import { SWRGlobalState } from './global-state' import type { Cache, State, GlobalState } from '../types' const EMPTY_CACHE = {} +const INITIAL_CACHE: Record = {} export const noop = () => {} // Using noop() as the undefined value as undefined can be replaced @@ -40,10 +41,27 @@ export const createCacheHelper = >( (info: T) => { if (!isUndefined(key)) { const prev = cache.get(key) + + // Before writing to the store, we keep the value in the initial cache + // if it's not there yet. + if (!(key in INITIAL_CACHE)) { + INITIAL_CACHE[key] = prev + } + state[5](key, mergeObjects(prev, info), prev || EMPTY_CACHE) } }, // Subscriber - state[6] + state[6], + // Get server cache snapshot + () => { + if (!isUndefined(key)) { + // If the cache was updated on the client, we return the stored initial value. + if (key in INITIAL_CACHE) return INITIAL_CACHE[key] + } + + // If we haven't done any client-side updates, we return the current value. + return (cache.get(key) || EMPTY_CACHE) as T + } ] as const } diff --git a/core/use-swr.ts b/core/use-swr.ts index a1c086880..b7e17f5bf 100644 --- a/core/use-swr.ts +++ b/core/use-swr.ts @@ -92,13 +92,14 @@ export const useSWRHandler = ( const getConfig = () => configRef.current const isActive = () => getConfig().isVisible() && getConfig().isOnline() - const [getCache, setCache, subscribeCache] = createCacheHelper< - Data, - State & { - // The original key arguments. - _k?: Key - } - >(cache, key) + const [getCache, setCache, subscribeCache, getInitialCache] = + createCacheHelper< + Data, + State & { + // The original key arguments. + _k?: Key + } + >(cache, key) const stateDependencies = useRef({}).current @@ -142,9 +143,8 @@ export const useSWRHandler = ( return true })() - const getSelectedCache = () => { - const state = getCache() - + // Get the cache and merge it with expected states. + const getSelectedCache = (state: ReturnType) => { // We only select the needed fields from the state. const snapshot = mergeObjects(state) delete snapshot._k @@ -160,14 +160,20 @@ export const useSWRHandler = ( } } - let memorizedSnapshot = getSelectedCache() - - return () => { - const snapshot = getSelectedCache() - return isEqual(snapshot, memorizedSnapshot) - ? memorizedSnapshot - : (memorizedSnapshot = snapshot) - } + // To make sure that we are returning the same object reference to avoid + // unnecessary re-renders, we keep the previous snapshot and use deep + // comparison to check if we need to return a new one. + let memorizedSnapshot = getSelectedCache(getCache()) + + return [ + () => { + const newSnapshot = getSelectedCache(getCache()) + return isEqual(newSnapshot, memorizedSnapshot) + ? memorizedSnapshot + : (memorizedSnapshot = newSnapshot) + }, + () => getSelectedCache(getInitialCache()) + ] // eslint-disable-next-line react-hooks/exhaustive-deps }, [cache, key]) @@ -184,8 +190,8 @@ export const useSWRHandler = ( // eslint-disable-next-line react-hooks/exhaustive-deps [cache, key] ), - getSnapshot, - getSnapshot + getSnapshot[0], + getSnapshot[1] ) const isInitialMount = !initialMountedRef.current diff --git a/test/use-swr-streaming-ssr.test.tsx b/test/use-swr-streaming-ssr.test.tsx new file mode 100644 index 000000000..bc1b1249f --- /dev/null +++ b/test/use-swr-streaming-ssr.test.tsx @@ -0,0 +1,81 @@ +import React, { Suspense } from 'react' +import useSWR from 'swr' +import { + createKey, + createResponse, + renderWithConfig, + hydrateWithConfig, + mockConsoleForHydrationErrors, + sleep +} from './utils' + +describe('useSWR - streaming', () => { + afterEach(() => { + jest.clearAllMocks() + jest.restoreAllMocks() + }) + + it('should match ssr result when hydrating', async () => { + const ensureAndUnmock = mockConsoleForHydrationErrors() + + const key = createKey() + + // A block fetches the data and updates the cache. + function Block() { + const { data } = useSWR(key, () => createResponse('SWR', { delay: 10 })) + return
{data || 'undefined'}
+ } + + const container = document.createElement('div') + container.innerHTML = '
undefined
' + await hydrateWithConfig(, container) + ensureAndUnmock() + }) + + // NOTE: this test is skipped because it's not possible to test this behavior + // in JSDOM. We need to test this in a real browser. + it.skip('should match the ssr result when streaming and partially hydrating', async () => { + const key = createKey() + + const dataDuringHydration = {} + + // A block fetches the data and updates the cache. + function Block({ suspense, delay, id }) { + const { data } = useSWR(key, () => createResponse('SWR', { delay }), { + suspense + }) + + // The first render is always hydration in our case. + if (!dataDuringHydration[id]) { + dataDuringHydration[id] = data || 'undefined' + } + + return
{data || 'undefined'}
+ } + + // In this example, a will be hydrated first and b will still be streamed. + // When a is hydrated, it will update the client cache to SWR, and when + // b is being hydrated, it should NOT read that cache. + renderWithConfig( + <> + + + + + + ) + + // The SSR result will always be 2 undefined values because data fetching won't + // happen on the server: + //
undefined
+ //
undefined
+ + // Wait for streaming to finish. + await sleep(50) + + expect(dataDuringHydration).toEqual({ + a: 'undefined', + b: 'undefined' + }) + }) +}) diff --git a/test/utils.tsx b/test/utils.tsx index e25048767..b15f844bf 100644 --- a/test/utils.tsx +++ b/test/utils.tsx @@ -54,6 +54,18 @@ export const renderWithGlobalCache = ( return _renderWithConfig(element, { ...config }) } +export const hydrateWithConfig = ( + element: React.ReactElement, + container: HTMLElement, + config?: Parameters[1] +): ReturnType => { + const provider = () => new Map() + const TestSWRConfig = ({ children }: { children: React.ReactNode }) => ( + {children} + ) + return render(element, { container, wrapper: TestSWRConfig, hydrate: true }) +} + export const mockVisibilityHidden = () => { const mockVisibilityState = jest.spyOn(document, 'visibilityState', 'get') mockVisibilityState.mockImplementation(() => 'hidden') @@ -68,3 +80,26 @@ export async function executeWithoutBatching(fn: () => any) { await fn() global.IS_REACT_ACT_ENVIRONMENT = prev } + +export const mockConsoleForHydrationErrors = () => { + jest.spyOn(console, 'error').mockImplementation(() => {}) + return () => { + // It should not have any hydration warnings. + expect( + // @ts-expect-error + console.error.mock.calls.find(([err]) => { + return ( + err?.message?.includes( + 'Text content does not match server-rendered HTML.' + ) || + err?.message?.includes( + 'Hydration failed because the initial UI does not match what was rendered on the server.' + ) + ) + }) + ).toBeFalsy() + + // @ts-expect-error + console.error.mockRestore() + } +} From bea16ac3b16cce5d9be9170dd9d6b80c1dd1abc1 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Fri, 27 Jan 2023 22:53:44 +0100 Subject: [PATCH 2/3] mark as failing --- test/use-swr-streaming-ssr.test.tsx | 77 +++++++++++++++-------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/test/use-swr-streaming-ssr.test.tsx b/test/use-swr-streaming-ssr.test.tsx index bc1b1249f..d60219841 100644 --- a/test/use-swr-streaming-ssr.test.tsx +++ b/test/use-swr-streaming-ssr.test.tsx @@ -32,50 +32,53 @@ describe('useSWR - streaming', () => { ensureAndUnmock() }) - // NOTE: this test is skipped because it's not possible to test this behavior + // NOTE: this test is failing because it's not possible to test this behavior // in JSDOM. We need to test this in a real browser. - it.skip('should match the ssr result when streaming and partially hydrating', async () => { - const key = createKey() + it.failing( + 'should match the ssr result when streaming and partially hydrating', + async () => { + const key = createKey() - const dataDuringHydration = {} + const dataDuringHydration = {} - // A block fetches the data and updates the cache. - function Block({ suspense, delay, id }) { - const { data } = useSWR(key, () => createResponse('SWR', { delay }), { - suspense - }) + // A block fetches the data and updates the cache. + function Block({ suspense, delay, id }) { + const { data } = useSWR(key, () => createResponse('SWR', { delay }), { + suspense + }) - // The first render is always hydration in our case. - if (!dataDuringHydration[id]) { - dataDuringHydration[id] = data || 'undefined' - } + // The first render is always hydration in our case. + if (!dataDuringHydration[id]) { + dataDuringHydration[id] = data || 'undefined' + } - return
{data || 'undefined'}
- } + return
{data || 'undefined'}
+ } - // In this example, a will be hydrated first and b will still be streamed. - // When a is hydrated, it will update the client cache to SWR, and when - // b is being hydrated, it should NOT read that cache. - renderWithConfig( - <> - - - - - - ) + // In this example, a will be hydrated first and b will still be streamed. + // When a is hydrated, it will update the client cache to SWR, and when + // b is being hydrated, it should NOT read that cache. + renderWithConfig( + <> + + + + + + ) - // The SSR result will always be 2 undefined values because data fetching won't - // happen on the server: - //
undefined
- //
undefined
+ // The SSR result will always be 2 undefined values because data fetching won't + // happen on the server: + //
undefined
+ //
undefined
- // Wait for streaming to finish. - await sleep(50) + // Wait for streaming to finish. + await sleep(50) - expect(dataDuringHydration).toEqual({ - a: 'undefined', - b: 'undefined' - }) - }) + expect(dataDuringHydration).toEqual({ + a: 'undefined', + b: 'undefined' + }) + } + ) }) From e794b8112e894aebaea8e400eed5cfec2c9d0ede Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Fri, 27 Jan 2023 23:17:03 +0100 Subject: [PATCH 3/3] memorize server snapshot --- core/use-swr.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/use-swr.ts b/core/use-swr.ts index b7e17f5bf..b95754049 100644 --- a/core/use-swr.ts +++ b/core/use-swr.ts @@ -164,6 +164,7 @@ export const useSWRHandler = ( // unnecessary re-renders, we keep the previous snapshot and use deep // comparison to check if we need to return a new one. let memorizedSnapshot = getSelectedCache(getCache()) + const memorizedInitialSnapshot = getSelectedCache(getInitialCache()) return [ () => { @@ -172,7 +173,7 @@ export const useSWRHandler = ( ? memorizedSnapshot : (memorizedSnapshot = newSnapshot) }, - () => getSelectedCache(getInitialCache()) + () => memorizedInitialSnapshot ] // eslint-disable-next-line react-hooks/exhaustive-deps }, [cache, key])