From c3eca504bff780a742390913cf1db06d147aa172 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Sat, 16 Apr 2022 17:40:09 +0200 Subject: [PATCH] replace isFallback with isLoading state; optimize code --- infinite/index.ts | 4 +- src/types.ts | 7 ++- src/use-swr.ts | 115 +++++++++++++++------------------- src/utils/env.ts | 4 +- src/utils/helper.ts | 8 +-- src/utils/web-preset.ts | 34 +++++----- test/use-swr-cache.test.tsx | 4 +- test/use-swr-loading.test.tsx | 108 ++++++++++++++++++++++++++----- 8 files changed, 176 insertions(+), 108 deletions(-) diff --git a/infinite/index.ts b/infinite/index.ts index bd44be3fd..5505fdee8 100644 --- a/infinite/index.ts +++ b/infinite/index.ts @@ -267,8 +267,8 @@ export const infinite = ((useSWRNext: SWRHook) => get isValidating() { return swr.isValidating }, - get isFallback() { - return swr.isFallback + get isLoading() { + return swr.isLoading } } as SWRInfiniteResponse }) as unknown as Middleware diff --git a/src/types.ts b/src/types.ts index ccaad377d..3a30349c5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,4 +1,5 @@ import * as revalidateEvents from './constants' +import { defaultConfig } from './utils/config' export type FetcherResponse = Data | Promise export type BareFetcher = ( @@ -121,7 +122,8 @@ export type Middleware = ( ) => ( key: Key, fetcher: BareFetcher | null, - config: SWRConfiguration> + config: typeof defaultConfig & + SWRConfiguration> ) => SWRResponse type ArgumentsTuple = [any, ...unknown[]] | readonly [any, ...unknown[]] @@ -164,6 +166,7 @@ export type State = { data?: Data error?: Error isValidating?: boolean + isLoading?: boolean } export type MutatorFn = ( @@ -220,7 +223,7 @@ export interface SWRResponse { error: Error | undefined mutate: KeyedMutator isValidating: boolean - isFallback: boolean + isLoading: boolean } export type KeyLoader = diff --git a/src/use-swr.ts b/src/use-swr.ts index f8e816da3..bd6fe8fea 100644 --- a/src/use-swr.ts +++ b/src/use-swr.ts @@ -87,10 +87,10 @@ export const useSWRHandler = ( const getConfig = () => configRef.current const isActive = () => getConfig().isVisible() && getConfig().isOnline() - const [get, set] = createCacheHelper(cache, key) + const [getCache, setCache] = createCacheHelper(cache, key) // Get the current state that SWR should return. - const cached = get() + const cached = getCache() const cachedData = cached.data const fallback = isUndefined(fallbackData) ? config.fallback[key] @@ -103,7 +103,7 @@ export const useSWRHandler = ( // - Suspense mode and there's stale data for the initial render. // - Not suspense mode and there is no fallback data and `revalidateIfStale` is enabled. // - `revalidateIfStale` is enabled but `data` is not defined. - const shouldRevalidate = () => { + const shouldDoInitialRevalidation = (() => { // If `revalidateOnMount` is set, we take the value directly. if (isInitialMount && !isUndefined(revalidateOnMount)) return revalidateOnMount @@ -119,22 +119,24 @@ export const useSWRHandler = ( // If there is no stale data, we need to revalidate on mount; // If `revalidateIfStale` is set to true, we will always revalidate. return isUndefined(data) || config.revalidateIfStale - } - - // Resolve the current validating state. - const resolveValidating = () => { - if (!key || !fetcher) return false - if (cached.isValidating) return true - - // If it's not mounted yet and it should revalidate on mount, revalidate. - return isInitialMount && shouldRevalidate() - } - const isValidating = resolveValidating() + })() + + // Resolve the default validating state: + // If it's able to validate, and it should revalidate on mount, this will be true. + const defaultValidatingState = !!( + key && + fetcher && + isInitialMount && + shouldDoInitialRevalidation + ) + const isValidating = cached.isValidating || defaultValidatingState + const isLoading = cached.isLoading || defaultValidatingState const currentState = { data, error, - isValidating + isValidating, + isLoading } const [stateRef, stateDependencies, setState] = useStateWithDeps(currentState) @@ -171,6 +173,18 @@ export const useSWRHandler = ( key === keyRef.current && initialMountedRef.current + // The final state object when request finishes. + const finalState: State = { + isValidating: false, + isLoading: false + } + const finishRequestAndUpdateState = () => { + setCache(finalState) + // We can only set state if it's safe (still mounted with the same key). + if (isCurrentKeyMounted()) { + setState(finalState) + } + } const cleanupState = () => { // Check if it's still the same request before deleting. const requestInfo = FETCH[key] @@ -179,19 +193,15 @@ export const useSWRHandler = ( } } - // The new state object when request finishes. - const newState: State = { isValidating: false } - const finishRequestAndUpdateState = () => { - set({ isValidating: false }) - // We can only set state if it's safe (still mounted with the same key). - if (isCurrentKeyMounted()) { - setState(newState) - } - } - // Start fetching. Change the `isValidating` state, update the cache. - set({ isValidating: true }) - setState({ isValidating: true }) + const initialState: State = { isValidating: true } + // It is in the `isLoading` state, if and only if there is no cached data. + // This bypasses fallback data and laggy data. + if (isUndefined(getCache().data)) { + initialState.isLoading = true + } + setCache(initialState) + setState(initialState) try { if (shouldStartNewRequest) { @@ -203,7 +213,7 @@ export const useSWRHandler = ( // If no cache being rendered currently (it shows a blank page), // we trigger the loading slow event. - if (config.loadingTimeout && isUndefined(get().data)) { + if (config.loadingTimeout && isUndefined(getCache().data)) { setTimeout(() => { if (loading && isCurrentKeyMounted()) { getConfig().onLoadingSlow(key, config) @@ -246,8 +256,7 @@ export const useSWRHandler = ( } // Clear error. - set({ error: UNDEFINED }) - newState.error = UNDEFINED + finalState.error = UNDEFINED // If there're other mutations(s), overlapped with the current revalidation: // case 1: @@ -283,19 +292,13 @@ export const useSWRHandler = ( // Deep compare with latest state to avoid extra re-renders. // For local state, compare and assign. if (!compare(stateRef.current.data, newData)) { - newState.data = newData + finalState.data = newData } else { - // data and newData are deeply equal - // it should be safe to broadcast the stale data - newState.data = stateRef.current.data + // `data` and `newData` are deeply equal (serialized value). + // So it should be safe to broadcast the stale data to keep referential equality (===). + finalState.data = stateRef.current.data // At the end of this function, `broadcastState` invokes the `onStateUpdate` function, - // which takes care of avoiding the re-render - } - - // For global state, it's possible that the key has changed. - // https://github.com/vercel/swr/pull/1058 - if (!compare(get().data, newData)) { - set({ data: newData }) + // which takes care of avoiding the re-render. } // Trigger the successful callback if it's the original request. @@ -313,8 +316,7 @@ export const useSWRHandler = ( // Not paused, we continue handling the error. Otherwise discard it. if (!currentConfig.isPaused()) { // Get a new error, don't use deep comparison for errors. - set({ error: err }) - newState.error = err as Error + finalState.error = err as Error // Error event and retry logic. Only for the actual request, not // deduped ones. @@ -354,7 +356,7 @@ export const useSWRHandler = ( // Here is the source of the request, need to tell all other hooks to // update their states. if (isCurrentKeyMounted() && shouldStartNewRequest) { - broadcastState(cache, key, { ...newState, isValidating: false }) + broadcastState(cache, key, finalState) } return true @@ -396,7 +398,6 @@ export const useSWRHandler = ( useIsomorphicLayoutEffect(() => { if (!key) return - const keyChanged = key !== keyRef.current const softRevalidate = revalidate.bind(UNDEFINED, WITH_DEDUPE) // Expose state updater to global event listeners. So we can update hook's @@ -451,18 +452,8 @@ export const useSWRHandler = ( keyRef.current = key initialMountedRef.current = true - // When `key` updates, reset the state to the initial value - // and trigger a rerender if necessary. - if (keyChanged) { - setState({ - data, - error, - isValidating - }) - } - // Trigger a revalidation. - if (shouldRevalidate()) { + if (shouldDoInitialRevalidation) { if (isUndefined(data) || IS_SERVER) { // Revalidate immediately. softRevalidate() @@ -480,7 +471,7 @@ export const useSWRHandler = ( unsubUpdate() unsubEvents() } - }, [key, revalidate]) + }, [key]) // Polling useIsomorphicLayoutEffect(() => { @@ -524,7 +515,7 @@ export const useSWRHandler = ( timer = -1 } } - }, [refreshInterval, refreshWhenHidden, refreshWhenOffline, revalidate]) + }, [refreshInterval, refreshWhenHidden, refreshWhenOffline, key]) // Display debug info in React DevTools. useDebugValue(data) @@ -555,11 +546,9 @@ export const useSWRHandler = ( stateDependencies.isValidating = true return isValidating }, - get isFallback() { - stateDependencies.data = true - // `isFallback` is only true when we are displaying a value other than - // the cached one. - return data !== cachedData + get isLoading() { + stateDependencies.isLoading = true + return isLoading } } as SWRResponse } diff --git a/src/utils/env.ts b/src/utils/env.ts index c67ff8cae..d21534ac1 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -1,7 +1,7 @@ import { useEffect, useLayoutEffect } from 'react' -import { hasRequestAnimationFrame, hasWindow } from './helper' +import { hasRequestAnimationFrame, isWindowDefined } from './helper' -export const IS_SERVER = !hasWindow() || 'Deno' in window +export const IS_SERVER = !isWindowDefined || 'Deno' in window // Polyfill requestAnimationFrame export const rAF = (f: (...args: any[]) => void) => diff --git a/src/utils/helper.ts b/src/utils/helper.ts index 68eb108f1..85c2bfa85 100644 --- a/src/utils/helper.ts +++ b/src/utils/helper.ts @@ -1,7 +1,7 @@ export const noop = () => {} // Using noop() as the undefined value as undefined can possibly be replaced -// by something else. Prettier ignore and extra parentheses are necessary here +// by something else. Prettier ignore and extra parentheses are necessary here // to ensure that tsc doesn't remove the __NOINLINE__ comment. // prettier-ignore export const UNDEFINED = (/*#__NOINLINE__*/ noop()) as undefined @@ -15,7 +15,7 @@ export const mergeObjects = (a: any, b: any) => OBJECT.assign({}, a, b) const STR_UNDEFINED = 'undefined' // NOTE: Use function to guarantee it's re-evaluated between jsdom and node runtime for tests. -export const hasWindow = () => typeof window != STR_UNDEFINED -export const hasDocument = () => typeof document != STR_UNDEFINED +export const isWindowDefined = typeof window != STR_UNDEFINED +export const isDocumentDefined = typeof document != STR_UNDEFINED export const hasRequestAnimationFrame = () => - hasWindow() && typeof window['requestAnimationFrame'] != STR_UNDEFINED + isWindowDefined && typeof window['requestAnimationFrame'] != STR_UNDEFINED diff --git a/src/utils/web-preset.ts b/src/utils/web-preset.ts index 27b467669..2877543aa 100644 --- a/src/utils/web-preset.ts +++ b/src/utils/web-preset.ts @@ -1,5 +1,5 @@ import { ProviderConfiguration } from '../types' -import { isUndefined, noop, hasWindow, hasDocument } from './helper' +import { isUndefined, noop, isWindowDefined, isDocumentDefined } from './helper' /** * Due to bug https://bugs.chromium.org/p/chromium/issues/detail?id=678075, @@ -11,34 +11,30 @@ import { isUndefined, noop, hasWindow, hasDocument } from './helper' let online = true const isOnline = () => online -const hasWin = hasWindow() -const hasDoc = hasDocument() - // For node and React Native, `add/removeEventListener` doesn't exist on window. -const onWindowEvent = - hasWin && window.addEventListener - ? window.addEventListener.bind(window) - : noop -const onDocumentEvent = hasDoc ? document.addEventListener.bind(document) : noop -const offWindowEvent = - hasWin && window.removeEventListener - ? window.removeEventListener.bind(window) - : noop -const offDocumentEvent = hasDoc - ? document.removeEventListener.bind(document) - : noop +const [onWindowEvent, offWindowEvent] = + isWindowDefined && window.addEventListener + ? [ + window.addEventListener.bind(window), + window.removeEventListener.bind(window) + ] + : [noop, noop] const isVisible = () => { - const visibilityState = hasDoc && document.visibilityState + const visibilityState = isDocumentDefined && document.visibilityState return isUndefined(visibilityState) || visibilityState !== 'hidden' } const initFocus = (callback: () => void) => { // focus revalidate - onDocumentEvent('visibilitychange', callback) + if (isDocumentDefined) { + document.addEventListener('visibilitychange', callback) + } onWindowEvent('focus', callback) return () => { - offDocumentEvent('visibilitychange', callback) + if (isDocumentDefined) { + document.removeEventListener('visibilitychange', callback) + } offWindowEvent('focus', callback) } } diff --git a/test/use-swr-cache.test.tsx b/test/use-swr-cache.test.tsx index b50851989..23c4208e6 100644 --- a/test/use-swr-cache.test.tsx +++ b/test/use-swr-cache.test.tsx @@ -198,13 +198,13 @@ describe('useSWR - cache provider', () => { it('should support fallback values with custom provider', async () => { const key = createKey() function Page() { - const { data, isFallback } = useSWR(key, async () => { + const { data, isLoading } = useSWR(key, async () => { await sleep(10) return 'data' }) return ( <> - {String(data)},{String(isFallback)} + {String(data)},{String(isLoading)} ) } diff --git a/test/use-swr-loading.test.tsx b/test/use-swr-loading.test.tsx index 625195a74..08f170e5b 100644 --- a/test/use-swr-loading.test.tsx +++ b/test/use-swr-loading.test.tsx @@ -10,7 +10,7 @@ import { } from './utils' describe('useSWR - loading', () => { - it('should return loading state', async () => { + it('should return validating state', async () => { let renderCount = 0 const key = createKey() function Page() { @@ -18,13 +18,13 @@ describe('useSWR - loading', () => { renderCount++ return (
- hello, {data}, {isValidating ? 'loading' : 'ready'} + hello, {data}, {isValidating ? 'validating' : 'ready'}
) } renderWithConfig() - screen.getByText('hello, , loading') + screen.getByText('hello, , validating') await screen.findByText('hello, data, ready') // data isValidating @@ -33,6 +33,29 @@ describe('useSWR - loading', () => { expect(renderCount).toEqual(2) }) + it('should return loading state', async () => { + let renderCount = 0 + const key = createKey() + function Page() { + const { data, isLoading } = useSWR(key, () => createResponse('data')) + renderCount++ + return ( +
+ hello, {data}, {isLoading ? 'loading' : 'ready'} +
+ ) + } + + renderWithConfig() + screen.getByText('hello, , loading') + + await screen.findByText('hello, data, ready') + // data isLoading + // -> undefined, true + // -> data, false + expect(renderCount).toEqual(2) + }) + it('should avoid extra rerenders', async () => { let renderCount = 0 const key = createKey() @@ -138,10 +161,10 @@ describe('useSWR - loading', () => { } renderWithConfig() - screen.getByText('data,error,isFallback,isValidating,mutate') + screen.getByText('data,error,isLoading,isValidating,mutate') }) - it('should sync loading states', async () => { + it('should sync validating states', async () => { const key = createKey() const fetcher = jest.fn() @@ -150,7 +173,7 @@ describe('useSWR - loading', () => { fetcher() return 'foo' }) - return isValidating ? <>loading : <>stopped + return isValidating ? <>validating : <>stopped } function Page() { @@ -162,13 +185,13 @@ describe('useSWR - loading', () => { } renderWithConfig() - screen.getByText('loading,loading') + screen.getByText('validating,validating') await nextTick() screen.getByText('stopped,stopped') expect(fetcher).toBeCalledTimes(1) }) - it('should sync all loading states if errored', async () => { + it('should sync all validating states if errored', async () => { const key = createKey() function Foo() { @@ -176,7 +199,7 @@ describe('useSWR - loading', () => { throw new Error(key) }) - return isValidating ? <>loading : <>stopped + return isValidating ? <>validating : <>stopped } function Page() { @@ -188,12 +211,12 @@ describe('useSWR - loading', () => { } renderWithConfig() - screen.getByText('loading,loading') + screen.getByText('validating,validating') await nextTick() screen.getByText('stopped,stopped') }) - it('should sync all loading states if errored but paused', async () => { + it('should sync all validating states if errored but paused', async () => { const key = createKey() let paused = false @@ -207,7 +230,7 @@ describe('useSWR - loading', () => { dedupingInterval: 0 }) - return isValidating ? <>loading : <>stopped + return isValidating ? <>validating : <>stopped } function Page() { @@ -222,13 +245,13 @@ describe('useSWR - loading', () => { } renderWithConfig() - screen.getByText('loading,') + screen.getByText('validating,') await act(() => sleep(70)) screen.getByText('stopped,') fireEvent.click(screen.getByText('start')) await act(() => sleep(20)) - screen.getByText('loading,loading') + screen.getByText('validating,validating') // Pause before it resolves paused = true @@ -237,4 +260,61 @@ describe('useSWR - loading', () => { // They should both stop screen.getByText('stopped,stopped') }) + + it('should not trigger loading state when revalidating', async () => { + const key = createKey() + let renderCount = 0 + function Page() { + const { isLoading, isValidating, mutate } = useSWR(key, () => + createResponse('data', { delay: 10 }) + ) + renderCount++ + return ( +
+
+ {isLoading ? 'loading' : 'ready'}, + {isValidating ? 'validating' : 'ready'} +
+ +
+ ) + } + + renderWithConfig() + screen.getByText('loading,validating') + await screen.findByText('ready,ready') + + fireEvent.click(screen.getByText('revalidate')) + screen.getByText('ready,validating') + await screen.findByText('ready,ready') + + // isValidating: true -> false -> true -> false + expect(renderCount).toBe(4) + }) + + it('should trigger loading state when changing the key', async () => { + function Page() { + const [key, setKey] = React.useState(createKey) + const { isLoading, isValidating } = useSWR(key, () => + createResponse('data', { delay: 10 }) + ) + return ( +
+
+ {isLoading ? 'loading' : 'ready'}, + {isValidating ? 'validating' : 'ready'} +
+ +
+ ) + } + + renderWithConfig() + screen.getByText('loading,validating') + await screen.findByText('ready,ready') + + fireEvent.click(screen.getByText('update key')) + screen.getByText('loading,validating') + await screen.findByText('ready,ready') + }) })