From 6d46633d46dcbd0f2a948aa7ababc0514aa2a44d Mon Sep 17 00:00:00 2001 From: Niek Date: Sat, 5 Sep 2020 11:42:09 +0200 Subject: [PATCH] refactor: cleanup to reduce file size and add tests --- .babelrc | 20 +++- .eslintrc | 3 +- package.json | 1 + src/core/query.ts | 59 +++++----- src/core/queryCache.ts | 28 ++--- src/core/queryObserver.ts | 50 ++++----- src/core/tests/utils.test.tsx | 39 ++++++- src/core/utils.ts | 18 ++-- src/hydration/tests/hydration.test.tsx | 6 +- src/react/ReactQueryCacheProvider.tsx | 2 +- .../tests/ReactQueryCacheProvider.test.tsx | 6 +- src/react/tests/ssr.test.tsx | 58 +++++----- src/react/tests/useInfiniteQuery.test.tsx | 9 +- src/react/tests/useMutation.test.tsx | 8 +- src/react/tests/usePaginatedQuery.test.tsx | 31 +++--- src/react/tests/useQuery.test.tsx | 102 +++++++++++++++--- src/react/useMutation.ts | 73 +++++++------ src/react/utils.ts | 6 -- tsconfig.json | 1 + yarn.lock | 10 +- 20 files changed, 324 insertions(+), 206 deletions(-) diff --git a/.babelrc b/.babelrc index 0ff1ff4eeb..59d8fd903a 100644 --- a/.babelrc +++ b/.babelrc @@ -11,7 +11,15 @@ "@babel/preset-typescript", "@babel/react" ], - "plugins": ["babel-plugin-transform-async-to-promises"], + "plugins": [ + [ + "const-enum", + { + "transform": "constObject" + } + ], + "babel-plugin-transform-async-to-promises" + ], "env": { "test": { "presets": [ @@ -24,7 +32,15 @@ } ] ], - "plugins": ["babel-plugin-transform-async-to-promises"] + "plugins": [ + [ + "const-enum", + { + "transform": "constObject" + } + ], + "babel-plugin-transform-async-to-promises" + ] } } } diff --git a/.eslintrc b/.eslintrc index 9fcf330faf..70c35c1b54 100644 --- a/.eslintrc +++ b/.eslintrc @@ -25,6 +25,7 @@ { "ignoreParameters": true } - ] + ], + "no-shadow": "error" } } diff --git a/package.json b/package.json index 93825fa3a6..4a22abb0fc 100644 --- a/package.json +++ b/package.json @@ -63,6 +63,7 @@ "@typescript-eslint/parser": "^3.6.1", "babel-eslint": "^10.1.0", "babel-jest": "^26.0.1", + "babel-plugin-const-enum": "^1.0.1", "babel-plugin-transform-async-to-promises": "^0.8.15", "cross-env": "^7.0.2", "eslint": "7.x", diff --git a/src/core/query.ts b/src/core/query.ts index 14d076b4bd..4007fd0033 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -9,6 +9,7 @@ import { isDocumentVisible, isOnline, isServer, + isValidTimeout, noop, replaceEqualDeep, sleep, @@ -26,14 +27,6 @@ import { QueryObserver, UpdateListener } from './queryObserver' // TYPES -interface QueryInitConfig { - queryCache: QueryCache - queryKey: ArrayQueryKey - queryHash: string - config: QueryConfig - notifyGlobalListeners: (query: Query) => void -} - export interface QueryState { canFetchMore?: boolean data?: TResult @@ -65,11 +58,11 @@ export interface RefetchOptions { throwOnError?: boolean } -export enum ActionType { - Failed = 'Failed', - Fetch = 'Fetch', - Success = 'Success', - Error = 'Error', +const enum ActionType { + Failed, + Fetch, + Success, + Error, } interface FailedAction { @@ -114,17 +107,19 @@ export class Query { private cancelFetch?: () => void private continueFetch?: () => void private isTransportCancelable?: boolean - private notifyGlobalListeners: (query: Query) => void - - constructor(init: QueryInitConfig) { - this.config = init.config - this.queryCache = init.queryCache - this.queryKey = init.queryKey - this.queryHash = init.queryHash - this.notifyGlobalListeners = init.notifyGlobalListeners + + constructor( + queryKey: ArrayQueryKey, + queryHash: string, + config: QueryConfig + ) { + this.config = config + this.queryKey = queryKey + this.queryHash = queryHash + this.queryCache = config.queryCache! this.observers = [] - this.state = getDefaultState(init.config) - this.cacheTime = init.config.cacheTime! + this.state = getDefaultState(config) + this.cacheTime = config.cacheTime! this.scheduleGc() } @@ -140,7 +135,7 @@ export class Query { observer.onQueryUpdate(action) }) - this.notifyGlobalListeners(this) + this.queryCache.notifyGlobalListeners(this) } private scheduleGc(): void { @@ -150,7 +145,7 @@ export class Query { this.clearGcTimeout() - if (this.cacheTime === Infinity || this.observers.length > 0) { + if (this.observers.length > 0 || !isValidTimeout(this.cacheTime)) { return } @@ -222,7 +217,7 @@ export class Query { } isStale(): boolean { - return this.observers.some(observer => observer.isStale()) + return this.observers.some(observer => observer.getCurrentResult().isStale) } isStaleByTime(staleTime = 0): boolean { @@ -234,16 +229,16 @@ export class Query { onInteraction(type: 'focus' | 'online'): void { // Execute the first observer which is enabled, // stale and wants to refetch on this interaction. - const observer = this.observers.find( + const staleObserver = this.observers.find( observer => - observer.isStale() && + observer.getCurrentResult().isStale && observer.config.enabled && ((observer.config.refetchOnWindowFocus && type === 'focus') || (observer.config.refetchOnReconnect && type === 'online')) ) - if (observer) { - observer.fetch().catch(noop) + if (staleObserver) { + staleObserver.fetch().catch(noop) } // Continue any paused fetch @@ -280,9 +275,9 @@ export class Query { if (this.isTransportCancelable) { this.cancel() } - } - this.scheduleGc() + this.scheduleGc() + } } async refetch( diff --git a/src/core/queryCache.ts b/src/core/queryCache.ts index 96dfe5d124..31585a5da1 100644 --- a/src/core/queryCache.ts +++ b/src/core/queryCache.ts @@ -4,7 +4,7 @@ import { functionalUpdate, getQueryArgs, isDocumentVisible, - isObject, + isPlainObject, isOnline, isServer, } from './utils' @@ -77,18 +77,15 @@ export class QueryCache { constructor(config?: QueryCacheConfig) { this.config = config || {} - - // A frozen cache does not add new queries to the cache this.globalListeners = [] - this.queries = {} this.queriesArray = [] this.isFetching = 0 } - private notifyGlobalListeners(query?: Query) { + notifyGlobalListeners(query?: Query) { this.isFetching = this.getQueries().reduce( - (acc, query) => (query.state.isFetching ? acc + 1 : acc), + (acc, q) => (q.state.isFetching ? acc + 1 : acc), 0 ) @@ -228,16 +225,9 @@ export class QueryCache { return this.queries[queryHash] as Query } - const query = new Query({ - queryCache: this, - queryKey, - queryHash, - config, - notifyGlobalListeners: query => { - this.notifyGlobalListeners(query) - }, - }) + const query = new Query(queryKey, queryHash, config) + // A frozen cache does not add new queries to the cache if (!this.config.frozen) { this.queries[queryHash] = query this.queriesArray.push(query) @@ -291,7 +281,7 @@ export class QueryCache { ...args: any[] ): Promise { if ( - isObject(args[1]) && + isPlainObject(args[1]) && (args[1].hasOwnProperty('throwOnError') || args[1].hasOwnProperty('force')) ) { @@ -312,9 +302,11 @@ export class QueryCache { ...config, }) - let query try { - query = this.buildQuery(queryKey, configWithoutRetry) + const query = this.buildQuery( + queryKey, + configWithoutRetry + ) if (options?.force || query.isStaleByTime(config.staleTime)) { await query.fetch(undefined, configWithoutRetry) } diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index 74c88dcfa1..517bf8ea9f 100644 --- a/src/core/queryObserver.ts +++ b/src/core/queryObserver.ts @@ -1,4 +1,9 @@ -import { getStatusProps, isServer, isDocumentVisible } from './utils' +import { + getStatusProps, + isServer, + isDocumentVisible, + isValidTimeout, +} from './utils' import type { QueryResult, QueryObserverConfig } from './types' import type { Query, Action, FetchMoreOptions, RefetchOptions } from './query' import type { QueryCache } from './queryCache' @@ -90,18 +95,12 @@ export class QueryObserver { // Update refetch interval if needed if ( config.enabled !== prevConfig.enabled || - config.refetchInterval !== prevConfig.refetchInterval || - config.refetchIntervalInBackground !== - prevConfig.refetchIntervalInBackground + config.refetchInterval !== prevConfig.refetchInterval ) { this.updateRefetchInterval() } } - isStale(): boolean { - return this.currentResult.isStale - } - getCurrentQuery(): Query { return this.currentQuery } @@ -144,14 +143,6 @@ export class QueryObserver { } } - private updateIsStale(): void { - const isStale = this.currentQuery.isStaleByTime(this.config.staleTime) - if (isStale !== this.currentResult.isStale) { - this.updateResult() - this.notify() - } - } - private notify(): void { this.updateListener?.(this.currentResult) } @@ -163,19 +154,19 @@ export class QueryObserver { this.clearStaleTimeout() - const staleTime = this.config.staleTime || 0 - const { isStale, updatedAt } = this.currentResult - - if (isStale || staleTime === Infinity) { + if (this.currentResult.isStale || !isValidTimeout(this.config.staleTime)) { return } - const timeElapsed = Date.now() - updatedAt - const timeUntilStale = staleTime - timeElapsed + 1 + const timeElapsed = Date.now() - this.currentResult.updatedAt + const timeUntilStale = this.config.staleTime - timeElapsed + 1 const timeout = Math.max(timeUntilStale, 0) this.staleTimeoutId = setTimeout(() => { - this.updateIsStale() + if (!this.currentResult.isStale) { + this.currentResult = { ...this.currentResult, isStale: true } + this.notify() + } }, timeout) } @@ -186,12 +177,7 @@ export class QueryObserver { this.clearRefetchInterval() - if ( - !this.config.enabled || - !this.config.refetchInterval || - this.config.refetchInterval < 0 || - this.config.refetchInterval === Infinity - ) { + if (!this.config.enabled || !isValidTimeout(this.config.refetchInterval)) { return } @@ -309,6 +295,8 @@ export class QueryObserver { } onQueryUpdate(action: Action): void { + const { type } = action + // Store current result and get new result const prevResult = this.currentResult this.updateResult() @@ -317,11 +305,11 @@ export class QueryObserver { // We need to check the action because the state could have // transitioned from success to success in case of `setQueryData`. - if (action.type === 'Success' && currentResult.isSuccess) { + if (type === 2) { config.onSuccess?.(currentResult.data!) config.onSettled?.(currentResult.data!, null) this.updateTimers() - } else if (action.type === 'Error' && currentResult.isError) { + } else if (type === 3) { config.onError?.(currentResult.error!) config.onSettled?.(undefined, currentResult.error!) this.updateTimers() diff --git a/src/core/tests/utils.test.tsx b/src/core/tests/utils.test.tsx index daf0da64e4..aca579458f 100644 --- a/src/core/tests/utils.test.tsx +++ b/src/core/tests/utils.test.tsx @@ -1,4 +1,9 @@ -import { deepEqual, replaceEqualDeep } from '../utils' +import { + deepEqual, + replaceEqualDeep, + deepIncludes, + isPlainObject, +} from '../utils' import { setConsole, queryCache } from '..' import { queryKey } from '../../react/tests/utils' @@ -29,6 +34,38 @@ describe('core/utils', () => { setConsole(console) }) + describe('isPlainObject', () => { + it('should return `true` for a plain object', () => { + expect(isPlainObject({})).toEqual(true) + }) + + it('should return `false` for an array', () => { + expect(isPlainObject([])).toEqual(false) + }) + + it('should return `false` for null', () => { + expect(isPlainObject(null)).toEqual(false) + }) + + it('should return `false` for undefined', () => { + expect(isPlainObject(undefined)).toEqual(false) + }) + }) + + describe('deepIncludes', () => { + it('should return `true` if a includes b', () => { + const a = { a: { b: 'b' }, c: 'c', d: [{ d: 'd ' }] } + const b = { a: { b: 'b' }, c: 'c', d: [] } + expect(deepIncludes(a, b)).toEqual(true) + }) + + it('should return `false` if a does not include b', () => { + const a = { a: { b: 'b' }, c: 'c', d: [] } + const b = { a: { b: 'b' }, c: 'c', d: [{ d: 'd ' }] } + expect(deepIncludes(a, b)).toEqual(false) + }) + }) + describe('deepEqual', () => { it('should return `true` for equal objects', () => { const a = { a: { b: 'b' }, c: 'c', d: [{ d: 'd ' }] } diff --git a/src/core/utils.ts b/src/core/utils.ts index 25d0c9ab5d..ca67602057 100644 --- a/src/core/utils.ts +++ b/src/core/utils.ts @@ -54,10 +54,10 @@ export function functionalUpdate( function stableStringifyReplacer(_key: string, value: any): unknown { if (typeof value === 'function') { - throw new Error('Cannot stringify non JSON value') + throw new Error() } - if (isObject(value)) { + if (isPlainObject(value)) { return Object.keys(value) .sort() .reduce((result, key) => { @@ -89,6 +89,10 @@ export function deepIncludes(a: any, b: any): boolean { return false } +export function isValidTimeout(value: any): value is number { + return typeof value === 'number' && value >= 0 && value !== Infinity +} + export function isDocumentVisible(): boolean { // document global can be unavailable in react native if (typeof document === 'undefined') { @@ -109,12 +113,12 @@ export function getQueryArgs( let config: QueryConfig | undefined let options: TOptions - if (isObject(args[0])) { + if (isPlainObject(args[0])) { queryKey = args[0].queryKey queryFn = args[0].queryFn config = args[0].config options = args[1] - } else if (isObject(args[1])) { + } else if (isPlainObject(args[1])) { queryKey = args[0] config = args[1] options = args[2] @@ -173,12 +177,8 @@ export function replaceEqualDeep(a: any, b: any): any { return b } -export function isObject(a: unknown): boolean { - return a && typeof a === 'object' && !Array.isArray(a) -} - // Copied from: https://github.com/jonschlinkert/is-plain-object -function isPlainObject(o: any): o is Object { +export function isPlainObject(o: any): o is Object { if (!hasObjectPrototype(o)) { return false } diff --git a/src/hydration/tests/hydration.test.tsx b/src/hydration/tests/hydration.test.tsx index 64e475bce1..1dcb83eb49 100644 --- a/src/hydration/tests/hydration.test.tsx +++ b/src/hydration/tests/hydration.test.tsx @@ -134,8 +134,7 @@ describe('dehydration and rehydration', () => { // Exact shape is not important here, just that staleTime and cacheTime // (and any future other config) is not included in it const dehydratedQuery = dehydrated?.queries.find( - dehydratedQuery => - (dehydratedQuery?.config?.queryKey as Array)[0] === 'string' + query => (query?.config?.queryKey as Array)[0] === 'string' ) expect(dehydratedQuery).toBeTruthy() expect(dehydratedQuery?.config.cacheTime).toBe(undefined) @@ -180,8 +179,7 @@ describe('dehydration and rehydration', () => { // This is testing implementation details that can change and are not // part of the public API, but is important for keeping the payload small const dehydratedQuery = dehydrated?.queries.find( - dehydratedQuery => - (dehydratedQuery?.config?.queryKey as Array)[0] === 'string' + query => (query?.config?.queryKey as Array)[0] === 'string' ) expect(dehydratedQuery).toBeUndefined() diff --git a/src/react/ReactQueryCacheProvider.tsx b/src/react/ReactQueryCacheProvider.tsx index 99265538ce..aa7541a566 100644 --- a/src/react/ReactQueryCacheProvider.tsx +++ b/src/react/ReactQueryCacheProvider.tsx @@ -7,7 +7,7 @@ import { } from '../core' import { QueryCache } from '../core/queryCache' -export const queryCacheContext = React.createContext(defaultQueryCache) +const queryCacheContext = React.createContext(defaultQueryCache) export const useQueryCache = () => React.useContext(queryCacheContext) diff --git a/src/react/tests/ReactQueryCacheProvider.test.tsx b/src/react/tests/ReactQueryCacheProvider.test.tsx index 9b9b395839..20e6c8e313 100644 --- a/src/react/tests/ReactQueryCacheProvider.test.tsx +++ b/src/react/tests/ReactQueryCacheProvider.test.tsx @@ -148,11 +148,11 @@ describe('ReactQueryCacheProvider', () => { const customCache = makeQueryCache() function Page() { - const queryCache = useQueryCache() + const contextCache = useQueryCache() useEffect(() => { - caches.push(queryCache) - }, [queryCache]) + caches.push(contextCache) + }, [contextCache]) const { data } = useQuery(key, async () => { await sleep(10) diff --git a/src/react/tests/ssr.test.tsx b/src/react/tests/ssr.test.tsx index 68f4b1aff4..5178d17241 100644 --- a/src/react/tests/ssr.test.tsx +++ b/src/react/tests/ssr.test.tsx @@ -34,19 +34,19 @@ describe('Server Side Rendering', () => { it('created caches should be unfrozen by default', async () => { const key = queryKey() - const queryCache = makeQueryCache() + const cache = makeQueryCache() const fetchFn = () => Promise.resolve('data') - const data = await queryCache.prefetchQuery(key, fetchFn) + const data = await cache.prefetchQuery(key, fetchFn) expect(data).toBe('data') - expect(queryCache.getQuery(key)).toBeTruthy() + expect(cache.getQuery(key)).toBeTruthy() }) describe('frozen cache', () => { it('should not trigger fetch', () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: true }) + const cache = makeQueryCache({ frozen: true }) const queryFn = jest.fn() function Page() { @@ -62,7 +62,7 @@ describe('Server Side Rendering', () => { } const markup = renderToString( - + ) @@ -74,20 +74,20 @@ describe('Server Side Rendering', () => { it('should not add initialData to the cache', () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: true }) + const cache = makeQueryCache({ frozen: true }) function Page() { const [page, setPage] = React.useState(1) const { resolvedData } = usePaginatedQuery( [key, page], - async (_queryName: string, page: number) => { - return page + async (_: string, pageArg: number) => { + return pageArg }, { initialData: 1 } ) return ( - +

{resolvedData}

 @@ -96,18 +96,18 @@ describe('Server Side Rendering', () => { renderToString() - expect(queryCache.getQueries().length).toEqual(0) + expect(cache.getQueries().length).toEqual(0) }) it('should not add prefetched data to the cache', async () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: true }) + const cache = makeQueryCache({ frozen: true }) const fetchFn = () => Promise.resolve('data') - const data = await queryCache.prefetchQuery(key, fetchFn) + const data = await cache.prefetchQuery(key, fetchFn) expect(data).toBe('data') - expect(queryCache.getQuery(key)).toBeFalsy() + expect(cache.getQuery(key)).toBeFalsy() }) }) @@ -115,7 +115,7 @@ describe('Server Side Rendering', () => { it('should not trigger fetch', () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: false }) + const cache = makeQueryCache({ frozen: false }) const queryFn = jest.fn() function Page() { @@ -131,7 +131,7 @@ describe('Server Side Rendering', () => { } const markup = renderToString( - + ) @@ -143,18 +143,18 @@ describe('Server Side Rendering', () => { it('should add prefetched data to cache', async () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: false }) + const cache = makeQueryCache({ frozen: false }) const fetchFn = () => Promise.resolve('data') - const data = await queryCache.prefetchQuery(key, fetchFn) + const data = await cache.prefetchQuery(key, fetchFn) expect(data).toBe('data') - expect(queryCache.getQuery(key)?.state.data).toBe('data') + expect(cache.getQuery(key)?.state.data).toBe('data') }) it('should return existing data from the cache', async () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: false }) + const cache = makeQueryCache({ frozen: false }) const queryFn = jest.fn(() => sleep(10)) function Page() { @@ -169,10 +169,10 @@ describe('Server Side Rendering', () => { ) } - await queryCache.prefetchQuery(key, queryFn) + await cache.prefetchQuery(key, queryFn) const markup = renderToString( - + ) @@ -184,13 +184,13 @@ describe('Server Side Rendering', () => { it('should add initialData to the cache', () => { const key = queryKey() - const queryCache = makeQueryCache({ frozen: false }) + const cache = makeQueryCache({ frozen: false }) function Page() { const [page, setPage] = React.useState(1) const { resolvedData } = usePaginatedQuery( [key, page], - async (_queryName: string, page: number) => { - return page + async (_: string, pageArg: number) => { + return pageArg }, { initialData: 1 } ) @@ -204,12 +204,12 @@ describe('Server Side Rendering', () => { } renderToString( - + ) - const keys = queryCache.getQueries().map(query => query.queryHash) + const keys = cache.getQueries().map(query => query.queryHash) expect(keys).toEqual([`["${key}",1]`]) }) @@ -220,7 +220,7 @@ describe('Server Side Rendering', () => { // @ts-ignore const setTimeoutMock = jest.spyOn(global, 'setTimeout') - const queryCache = makeQueryCache({ frozen: false }) + const cache = makeQueryCache({ frozen: false }) const queryFn = jest.fn(() => Promise.resolve()) function Page() { @@ -235,10 +235,10 @@ describe('Server Side Rendering', () => { ) } - await queryCache.prefetchQuery(key, queryFn) + await cache.prefetchQuery(key, queryFn) const markup = renderToString( - + ) diff --git a/src/react/tests/useInfiniteQuery.test.tsx b/src/react/tests/useInfiniteQuery.test.tsx index f991efb535..86deb29d08 100644 --- a/src/react/tests/useInfiniteQuery.test.tsx +++ b/src/react/tests/useInfiniteQuery.test.tsx @@ -119,9 +119,9 @@ describe('useInfiniteQuery', () => { const state = useInfiniteQuery( [key, order], - async (_key, order, page = 0) => { + async (_key, orderArg, pageArg = 0) => { await sleep(10) - return `${page}-${order}` + return `${pageArg}-${orderArg}` }, { getFetchMore: (_lastGroup, _allGroups) => 1, @@ -765,7 +765,7 @@ describe('useInfiniteQuery', () => { const items = genItems(15) const limit = 3 - const fetchItems = async (cursor = 0, ts: number) => { + const fetchItemsWithLimit = async (cursor = 0, ts: number) => { await sleep(10) return { nextId: cursor + limit, @@ -787,7 +787,8 @@ describe('useInfiniteQuery', () => { refetch, } = useInfiniteQuery( key, - (_key, nextId = 0) => fetchItems(nextId, fetchCountRef.current++), + (_key, nextId = 0) => + fetchItemsWithLimit(nextId, fetchCountRef.current++), { getFetchMore: (lastGroup, _allGroups) => lastGroup.nextId, } diff --git a/src/react/tests/useMutation.test.tsx b/src/react/tests/useMutation.test.tsx index 07e7ed4c66..8d1e48318e 100644 --- a/src/react/tests/useMutation.test.tsx +++ b/src/react/tests/useMutation.test.tsx @@ -89,7 +89,7 @@ describe('useMutation', () => { function Page() { const [mutate] = useMutation( - async ({ count }: { count: number }) => Promise.resolve(count), + async (vars: { count: number }) => Promise.resolve(vars.count), { onSuccess: data => onSuccessMock(data), onSettled: data => onSettledMock(data), @@ -136,8 +136,10 @@ describe('useMutation', () => { function Page() { const [mutate] = useMutation( - ({ count }: { count: number }) => { - const error = new Error(`Expected mock error. All is well! ${count}`) + (vars: { count: number }) => { + const error = new Error( + `Expected mock error. All is well! ${vars.count}` + ) error.stack = '' return Promise.reject(error) }, diff --git a/src/react/tests/usePaginatedQuery.test.tsx b/src/react/tests/usePaginatedQuery.test.tsx index d54465216d..c12f11637a 100644 --- a/src/react/tests/usePaginatedQuery.test.tsx +++ b/src/react/tests/usePaginatedQuery.test.tsx @@ -11,13 +11,10 @@ describe('usePaginatedQuery', () => { const states: PaginatedQueryResult[] = [] function Page() { - const state = usePaginatedQuery( - [key, 1], - async (_queryName, page: number) => { - await sleep(10) - return page - } - ) + const state = usePaginatedQuery([key, 1], async (_, page: number) => { + await sleep(10) + return page + }) states.push(state) @@ -88,9 +85,9 @@ describe('usePaginatedQuery', () => { const [page, setPage] = React.useState(1) const { resolvedData = 'undefined' } = usePaginatedQuery( [key, page], - async (_queryName: string, page: number) => { + async (_: string, pageArg: number) => { await sleep(10) - return page + return pageArg } ) @@ -125,9 +122,9 @@ describe('usePaginatedQuery', () => { const { resolvedData } = usePaginatedQuery( [key, params], - async (_queryName: string, { page }: typeof params) => { + async (_: string, paramsArg: typeof params) => { await sleep(10) - return page + return paramsArg.page }, { initialData: 0 } ) @@ -167,9 +164,9 @@ describe('usePaginatedQuery', () => { const { resolvedData, status } = usePaginatedQuery( [key, params], - async (_queryName: string, { page }: typeof params) => { + async (_: string, paramsArg: typeof params) => { await sleep(10) - return page + return paramsArg.page }, { initialData: 0 } ) @@ -204,9 +201,9 @@ describe('usePaginatedQuery', () => { const [page, setPage] = React.useState(1) const { resolvedData = 'undefined' } = usePaginatedQuery( [key, searchTerm, page], - async (_queryName: string, searchTerm: string, page: number) => { + async (_: string, searchTermArg: string, pageArg: number) => { await sleep(10) - return `${searchTerm} ${page}` + return `${searchTermArg} ${pageArg}` }, { enabled: searchTerm, @@ -271,9 +268,9 @@ describe('usePaginatedQuery', () => { const { resolvedData } = usePaginatedQuery( [key, params], - async (_queryName: string, { page }: typeof params) => { + async (_: string, paramsArg: typeof params) => { await sleep(10) - return page + return paramsArg.page }, { initialData: 0, diff --git a/src/react/tests/useQuery.test.tsx b/src/react/tests/useQuery.test.tsx index ff091c2fc3..684ac832ba 100644 --- a/src/react/tests/useQuery.test.tsx +++ b/src/react/tests/useQuery.test.tsx @@ -297,6 +297,88 @@ describe('useQuery', () => { }) }) + it('should call onSuccess after a query has been fetched', async () => { + const key = queryKey() + const states: QueryResult[] = [] + const onSuccess = jest.fn() + + function Page() { + const state = useQuery(key, () => 'data', { onSuccess }) + states.push(state) + return null + } + + render() + + await waitFor(() => expect(states.length).toBe(2)) + expect(onSuccess).toHaveBeenCalledTimes(1) + expect(onSuccess).toHaveBeenCalledWith('data') + }) + + it('should call onError after a query has been fetched with an error', async () => { + const key = queryKey() + const states: QueryResult[] = [] + const onError = jest.fn() + const consoleMock = mockConsoleError() + + function Page() { + const state = useQuery(key, () => Promise.reject('error'), { + retry: false, + onError, + }) + states.push(state) + return null + } + + render() + + await waitFor(() => expect(states.length).toBe(2)) + expect(onError).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledWith('error') + consoleMock.mockRestore() + }) + + it('should call onSettled after a query has been fetched', async () => { + const key = queryKey() + const states: QueryResult[] = [] + const onSettled = jest.fn() + + function Page() { + const state = useQuery(key, () => 'data', { onSettled }) + states.push(state) + return null + } + + render() + + await waitFor(() => expect(states.length).toBe(2)) + expect(onSettled).toHaveBeenCalledTimes(1) + expect(onSettled).toHaveBeenCalledWith('data', null) + }) + + it('should call onSettled after a query has been fetched with an error', async () => { + const key = queryKey() + const states: QueryResult[] = [] + const onSettled = jest.fn() + const consoleMock = mockConsoleError() + + function Page() { + const state = useQuery(key, () => Promise.reject('error'), { + retry: false, + onSettled, + }) + states.push(state) + return null + } + + render() + + await waitFor(() => expect(states.length).toBe(2)) + expect(onSettled).toHaveBeenCalledTimes(1) + expect(onSettled).toHaveBeenCalledWith(undefined, 'error') + consoleMock.mockRestore() + }) + // https://github.com/tannerlinsley/react-query/issues/896 it('should fetch data in Strict mode when refetchOnMount is false', async () => { const key = queryKey() @@ -738,17 +820,13 @@ describe('useQuery', () => { render() - await waitFor(() => expect(states.length).toBe(3)) - - expect(states[0]).toMatchObject({ - isStale: true, - }) - expect(states[1]).toMatchObject({ - isStale: false, - }) - expect(states[2]).toMatchObject({ - isStale: true, - }) + await waitFor(() => + expect(states).toMatchObject([ + { isStale: true }, + { isStale: false }, + { isStale: true }, + ]) + ) }) it('should not re-render when a query status changes and notifyOnStatusChange is false', async () => { @@ -1134,7 +1212,7 @@ describe('useQuery', () => { [string] >(key, queryFn, { retryDelay: 1, - retry: (_failureCount, error) => error !== 'NoRetry', + retry: (_failureCount, err) => err !== 'NoRetry', }) return ( diff --git a/src/react/useMutation.ts b/src/react/useMutation.ts index 349cfbaccf..7826a70051 100644 --- a/src/react/useMutation.ts +++ b/src/react/useMutation.ts @@ -1,7 +1,7 @@ import React from 'react' import { useDefaultedMutationConfig } from './useDefaultedMutationConfig' -import { useGetLatest, useMountedCallback } from './utils' +import { useMountedCallback } from './utils' import { Console, uid, getStatusProps } from '../core/utils' import { QueryStatus, @@ -9,6 +9,7 @@ import { MutationFunction, MutationConfig, MutateConfig, + MutationResult, } from '../core/types' // TYPES @@ -25,11 +26,11 @@ interface State { isError: boolean } -enum ActionType { - Reset = 'Reset', - Loading = 'Loading', - Resolve = 'Resolve', - Reject = 'Reject', +const enum ActionType { + Reset, + Loading, + Resolve, + Reject, } interface ResetAction { @@ -58,11 +59,13 @@ type Action = // HOOK -const getDefaultState = (): State => ({ - ...getStatusProps(QueryStatus.Idle), - data: undefined, - error: null, -}) +function getDefaultState(): State { + return { + ...getStatusProps(QueryStatus.Idle), + data: undefined, + error: null, + } +} function mutationReducer( state: State, @@ -104,26 +107,26 @@ export function useMutation< config: MutationConfig = {} ): MutationResultPair { config = useDefaultedMutationConfig(config) - const getConfig = useGetLatest(config) const [state, unsafeDispatch] = React.useReducer( mutationReducer as Reducer, Action>, null, getDefaultState ) - const dispatch = useMountedCallback(unsafeDispatch) - const getMutationFn = useGetLatest(mutationFn) - const latestMutationRef = React.useRef() + const latestMutationFnRef = React.useRef(mutationFn) + latestMutationFnRef.current = mutationFn + const latestConfigRef = React.useRef(config) + latestConfigRef.current = config const mutate = React.useCallback( async ( variables?: TVariables, mutateConfig: MutateConfig = {} ): Promise => { - const config = getConfig() + const latestConfig = latestConfigRef.current const mutationId = uid() latestMutationRef.current = mutationId @@ -134,25 +137,26 @@ export function useMutation< try { dispatch({ type: ActionType.Loading }) - snapshotValue = (await config.onMutate?.(variables!)) as TSnapshot + snapshotValue = (await latestConfig.onMutate?.(variables!)) as TSnapshot - const data = await getMutationFn()(variables!) + const latestMutationFn = latestMutationFnRef.current + const data = await latestMutationFn(variables!) if (isLatest()) { dispatch({ type: ActionType.Resolve, data }) } - await config.onSuccess?.(data, variables!) + await latestConfig.onSuccess?.(data, variables!) await mutateConfig.onSuccess?.(data, variables!) - await config.onSettled?.(data, null, variables!) + await latestConfig.onSettled?.(data, null, variables!) await mutateConfig.onSettled?.(data, null, variables!) return data } catch (error) { Console.error(error) - await config.onError?.(error, variables!, snapshotValue!) + await latestConfig.onError?.(error, variables!, snapshotValue!) await mutateConfig.onError?.(error, variables!, snapshotValue!) - await config.onSettled?.( + await latestConfig.onSettled?.( undefined, error, variables!, @@ -169,25 +173,30 @@ export function useMutation< dispatch({ type: ActionType.Reject, error }) } - if (mutateConfig.throwOnError ?? config.throwOnError) { + if (mutateConfig.throwOnError || latestConfig.throwOnError) { throw error } } }, - [dispatch, getConfig, getMutationFn] + [dispatch] ) - const reset = React.useCallback(() => { - dispatch({ type: ActionType.Reset }) - }, [dispatch]) - React.useEffect(() => { - const { suspense, useErrorBoundary } = getConfig() - + const latestConfig = latestConfigRef.current + const { suspense, useErrorBoundary } = latestConfig if ((useErrorBoundary ?? suspense) && state.error) { throw state.error } - }, [getConfig, state.error]) + }, [state.error]) + + const reset = React.useCallback(() => { + dispatch({ type: ActionType.Reset }) + }, [dispatch]) + + const result: MutationResult = { + ...state, + reset, + } - return [mutate, { ...state, reset }] + return [mutate, result] } diff --git a/src/react/utils.ts b/src/react/utils.ts index d550e35d41..034b03f4cb 100644 --- a/src/react/utils.ts +++ b/src/react/utils.ts @@ -2,12 +2,6 @@ import React from 'react' import { isServer } from '../core/utils' -export function useGetLatest(obj: T): () => T { - const ref = React.useRef(obj) - ref.current = obj - return React.useCallback(() => ref.current, []) -} - function useIsMounted(): () => boolean { const mountedRef = React.useRef(false) const isMounted = React.useCallback(() => mountedRef.current, []) diff --git a/tsconfig.json b/tsconfig.json index 287685b3f9..eda3826e7b 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,7 @@ { "compilerOptions": { "esModuleInterop": true, + "isolatedModules": true, "jsx": "react", "lib": ["ESNext", "dom"], "target": "es5", diff --git a/yarn.lock b/yarn.lock index 11a14154cc..256de41636 100644 --- a/yarn.lock +++ b/yarn.lock @@ -713,7 +713,7 @@ dependencies: "@babel/helper-plugin-utils" "^7.10.1" -"@babel/plugin-syntax-typescript@^7.10.4": +"@babel/plugin-syntax-typescript@^7.10.4", "@babel/plugin-syntax-typescript@^7.3.3": version "7.10.4" resolved "https://registry.yarnpkg.com/@babel/plugin-syntax-typescript/-/plugin-syntax-typescript-7.10.4.tgz#2f55e770d3501e83af217d782cb7517d7bb34d25" integrity sha512-oSAEz1YkBCAKr5Yiq8/BNtvSAPwkp/IyUnwZogd8p+F0RuYQQrLeRUzIQhueQTTBy/F+a40uS7OFKxnkRvmvFQ== @@ -2151,6 +2151,14 @@ babel-jest@^26.0.1: graceful-fs "^4.2.4" slash "^3.0.0" +babel-plugin-const-enum@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/babel-plugin-const-enum/-/babel-plugin-const-enum-1.0.1.tgz#0d742faf9731be4f213c4d01d61fc4e93c44d159" + integrity sha512-6oGu63g1FS9psUPQyLCJM08ty6kGihGKTbzWGbAKHfUuCzCh7y9twh516cR6v0lM4d4NOoR+DgLb7uKVytyp6Q== + dependencies: + "@babel/helper-plugin-utils" "^7.0.0" + "@babel/plugin-syntax-typescript" "^7.3.3" + babel-plugin-dynamic-import-node@^2.3.3: version "2.3.3" resolved "https://registry.yarnpkg.com/babel-plugin-dynamic-import-node/-/babel-plugin-dynamic-import-node-2.3.3.tgz#84fda19c976ec5c6defef57f9427b3def66e17a3"