From 7204cdd0e408fce77cc8318aaf87cda0fe9c51d1 Mon Sep 17 00:00:00 2001 From: Roman Kofman <35267+rkofman@users.noreply.github.com> Date: Sat, 30 Mar 2024 14:36:25 -0700 Subject: [PATCH 1/5] proof of concept for handling bigint in useQuerySubscription --- .../src/query/defaultSerializeQueryArgs.ts | 32 ++++++++++++++++ .../toolkit/src/query/react/buildHooks.ts | 4 +- .../src/query/tests/buildHooks.test.tsx | 38 ++++++++++++++++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts index fcfd63aeb..cc5e95757 100644 --- a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts +++ b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts @@ -36,6 +36,38 @@ export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ return `${endpointName}(${serialized})` } +export const bigIntSafeSerializeQueryArgs: SerializeQueryArgs = ({ + endpointName, + queryArgs, +}) => { + let serialized = '' + + const cached = cache?.get(queryArgs) + + if (typeof cached === 'string') { + serialized = cached + } else { + const stringified = JSON.stringify(queryArgs, (key, value) => { + value = typeof value === 'bigint' ? { $bigint: value.toString() } : value + value = isPlainObject(value) + ? Object.keys(value) + .sort() + .reduce((acc, key) => { + acc[key] = (value as any)[key] + return acc + }, {}) + : value + return value + }) + if (isPlainObject(queryArgs)) { + cache?.set(queryArgs, stringified) + } + serialized = stringified + } + // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) + return `${endpointName}(${serialized})` +} + export type SerializeQueryArgs = (_: { queryArgs: QueryArgs endpointDefinition: EndpointDefinition diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index d2432acc9..9579e705b 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -45,7 +45,7 @@ import { import { shallowEqual } from 'react-redux' import type { BaseQueryFn } from '../baseQueryTypes' import type { SubscriptionSelectors } from '../core/buildMiddleware/types' -import { defaultSerializeQueryArgs } from '../defaultSerializeQueryArgs' +import { bigIntSafeSerializeQueryArgs } from '../defaultSerializeQueryArgs' import type { UninitializedValue } from './constants' import { UNINITIALIZED_VALUE } from './constants' import type { ReactHooksModuleOptions } from './module' @@ -768,7 +768,7 @@ export function buildHooks({ // so we can tell if _anything_ actually changed. Otherwise, we can end up // with a case where the query args did change but the serialization doesn't, // and then we never try to initiate a refetch. - defaultSerializeQueryArgs, + bigIntSafeSerializeQueryArgs, context.endpointDefinitions[name], name, ) diff --git a/packages/toolkit/src/query/tests/buildHooks.test.tsx b/packages/toolkit/src/query/tests/buildHooks.test.tsx index 2d5106971..2a1ffe051 100644 --- a/packages/toolkit/src/query/tests/buildHooks.test.tsx +++ b/packages/toolkit/src/query/tests/buildHooks.test.tsx @@ -100,7 +100,7 @@ const api = createApi({ getError: build.query({ query: () => '/error', }), - listItems: build.query({ + listItems: build.query({ serializeQueryArgs: ({ endpointName }) => { return endpointName }, @@ -151,6 +151,7 @@ beforeEach(() => { }) afterEach(() => { + nextItemId = 0 amount = 0 listenerMiddleware.clearListeners() }) @@ -630,7 +631,40 @@ describe('hooks tests', () => { test(`useQuery refetches when query args object changes even if serialized args don't change`, async () => { function ItemList() { const [pageNumber, setPageNumber] = useState(0) - const { data = [] } = api.useListItemsQuery({ pageNumber }) + const { data = [] } = api.useListItemsQuery({ + pageNumber: pageNumber, + }) + + const renderedItems = data.map((item) => ( +
  • ID: {item.id}
    • + )) + return ( +
    + +
      {renderedItems}
    +
    + ) + } + + render(, { wrapper: storeRef.wrapper }) + + await screen.findByText('ID: 0') + + await act(async () => { + screen.getByText('Next Page').click() + }) + + await screen.findByText('ID: 3') + }) + + test(`useQuery gracefully handles bigint types`, async () => { + function ItemList() { + const [pageNumber, setPageNumber] = useState(0) + const { data = [] } = api.useListItemsQuery({ + pageNumber: BigInt(pageNumber), + }) const renderedItems = data.map((item) => (
  • ID: {item.id}
    • From ea5ea71597248e79eb26c0757e949d0320037558 Mon Sep 17 00:00:00 2001 From: Roman Kofman <35267+rkofman@users.noreply.github.com> Date: Sat, 30 Mar 2024 14:52:10 -0700 Subject: [PATCH 2/5] update comments for increased locality to described behavior --- packages/toolkit/src/query/defaultSerializeQueryArgs.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts index cc5e95757..23d6136e0 100644 --- a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts +++ b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts @@ -18,6 +18,7 @@ export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ serialized = cached } else { const stringified = JSON.stringify(queryArgs, (key, value) => + // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) isPlainObject(value) ? Object.keys(value) .sort() @@ -32,7 +33,6 @@ export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ } serialized = stringified } - // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) return `${endpointName}(${serialized})` } @@ -48,7 +48,9 @@ export const bigIntSafeSerializeQueryArgs: SerializeQueryArgs = ({ serialized = cached } else { const stringified = JSON.stringify(queryArgs, (key, value) => { + // Translate bigint fields to a serializable value value = typeof value === 'bigint' ? { $bigint: value.toString() } : value + // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) value = isPlainObject(value) ? Object.keys(value) .sort() From e2dff717f5ed2ddff1ef29fd72df0ac0baaffdde Mon Sep 17 00:00:00 2001 From: Roman Kofman <35267+rkofman@users.noreply.github.com> Date: Sat, 30 Mar 2024 15:13:54 -0700 Subject: [PATCH 3/5] refactor to remove code duplication --- .../src/query/defaultSerializeQueryArgs.ts | 40 ++++--------------- .../toolkit/src/query/react/buildHooks.ts | 7 +++- 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts index 23d6136e0..20dd7db32 100644 --- a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts +++ b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts @@ -9,36 +9,7 @@ const cache: WeakMap | undefined = WeakMap export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ endpointName, queryArgs, -}) => { - let serialized = '' - - const cached = cache?.get(queryArgs) - - if (typeof cached === 'string') { - serialized = cached - } else { - const stringified = JSON.stringify(queryArgs, (key, value) => - // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) - isPlainObject(value) - ? Object.keys(value) - .sort() - .reduce((acc, key) => { - acc[key] = (value as any)[key] - return acc - }, {}) - : value, - ) - if (isPlainObject(queryArgs)) { - cache?.set(queryArgs, stringified) - } - serialized = stringified - } - return `${endpointName}(${serialized})` -} - -export const bigIntSafeSerializeQueryArgs: SerializeQueryArgs = ({ - endpointName, - queryArgs, + replacer, }) => { let serialized = '' @@ -48,8 +19,8 @@ export const bigIntSafeSerializeQueryArgs: SerializeQueryArgs = ({ serialized = cached } else { const stringified = JSON.stringify(queryArgs, (key, value) => { - // Translate bigint fields to a serializable value - value = typeof value === 'bigint' ? { $bigint: value.toString() } : value + // Use custom replacer first before applying key-sorting behavior: + value = replacer ? replacer(key, value) : value // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) value = isPlainObject(value) ? Object.keys(value) @@ -66,7 +37,6 @@ export const bigIntSafeSerializeQueryArgs: SerializeQueryArgs = ({ } serialized = stringified } - // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) return `${endpointName}(${serialized})` } @@ -74,10 +44,14 @@ export type SerializeQueryArgs = (_: { queryArgs: QueryArgs endpointDefinition: EndpointDefinition endpointName: string + // Allows for a custom stringify replacer while keeping key-sorting behavior. e.g. for serializing bigint. + replacer?: (key: string, value: any) => {} }) => ReturnType export type InternalSerializeQueryArgs = (_: { queryArgs: any endpointDefinition: EndpointDefinition endpointName: string + // Allows for a custom stringify replacer while keeping key-sorting behavior. e.g. for serializing bigint. + replacer?: (key: string, value: any) => {} }) => QueryCacheKey diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index 9579e705b..631bfd863 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -45,7 +45,7 @@ import { import { shallowEqual } from 'react-redux' import type { BaseQueryFn } from '../baseQueryTypes' import type { SubscriptionSelectors } from '../core/buildMiddleware/types' -import { bigIntSafeSerializeQueryArgs } from '../defaultSerializeQueryArgs' +import { defaultSerializeQueryArgs } from '../defaultSerializeQueryArgs' import type { UninitializedValue } from './constants' import { UNINITIALIZED_VALUE } from './constants' import type { ReactHooksModuleOptions } from './module' @@ -761,6 +761,8 @@ export function buildHooks({ subscriptionSelectorsRef.current = returnedValue as unknown as SubscriptionSelectors } + const bigIntReplacer = (_: string, value: any) => + typeof value === 'bigint' ? { $bigint: value.toString() } : value const stableArg = useStableQueryArgs( skip ? skipToken : arg, // Even if the user provided a per-endpoint `serializeQueryArgs` with @@ -768,7 +770,8 @@ export function buildHooks({ // so we can tell if _anything_ actually changed. Otherwise, we can end up // with a case where the query args did change but the serialization doesn't, // and then we never try to initiate a refetch. - bigIntSafeSerializeQueryArgs, + (params) => + defaultSerializeQueryArgs({ replacer: bigIntReplacer, ...params }), context.endpointDefinitions[name], name, ) From ae838b4c6a21ff11d560af4ed6f8362f7a467afa Mon Sep 17 00:00:00 2001 From: Roman Kofman <35267+rkofman@users.noreply.github.com> Date: Mon, 8 Apr 2024 10:32:02 -0500 Subject: [PATCH 4/5] remove replacer param in favor of handling bigints inside of defaultSerializeQueryArgs --- packages/toolkit/src/query/defaultSerializeQueryArgs.ts | 9 ++------- packages/toolkit/src/query/react/buildHooks.ts | 5 +---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts index 20dd7db32..0f5e5bbe2 100644 --- a/packages/toolkit/src/query/defaultSerializeQueryArgs.ts +++ b/packages/toolkit/src/query/defaultSerializeQueryArgs.ts @@ -9,7 +9,6 @@ const cache: WeakMap | undefined = WeakMap export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ endpointName, queryArgs, - replacer, }) => { let serialized = '' @@ -19,8 +18,8 @@ export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ serialized = cached } else { const stringified = JSON.stringify(queryArgs, (key, value) => { - // Use custom replacer first before applying key-sorting behavior: - value = replacer ? replacer(key, value) : value + // Handle bigints + value = typeof value === 'bigint' ? { $bigint: value.toString() } : value // Sort the object keys before stringifying, to prevent useQuery({ a: 1, b: 2 }) having a different cache key than useQuery({ b: 2, a: 1 }) value = isPlainObject(value) ? Object.keys(value) @@ -44,14 +43,10 @@ export type SerializeQueryArgs = (_: { queryArgs: QueryArgs endpointDefinition: EndpointDefinition endpointName: string - // Allows for a custom stringify replacer while keeping key-sorting behavior. e.g. for serializing bigint. - replacer?: (key: string, value: any) => {} }) => ReturnType export type InternalSerializeQueryArgs = (_: { queryArgs: any endpointDefinition: EndpointDefinition endpointName: string - // Allows for a custom stringify replacer while keeping key-sorting behavior. e.g. for serializing bigint. - replacer?: (key: string, value: any) => {} }) => QueryCacheKey diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index 631bfd863..d2432acc9 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -761,8 +761,6 @@ export function buildHooks({ subscriptionSelectorsRef.current = returnedValue as unknown as SubscriptionSelectors } - const bigIntReplacer = (_: string, value: any) => - typeof value === 'bigint' ? { $bigint: value.toString() } : value const stableArg = useStableQueryArgs( skip ? skipToken : arg, // Even if the user provided a per-endpoint `serializeQueryArgs` with @@ -770,8 +768,7 @@ export function buildHooks({ // so we can tell if _anything_ actually changed. Otherwise, we can end up // with a case where the query args did change but the serialization doesn't, // and then we never try to initiate a refetch. - (params) => - defaultSerializeQueryArgs({ replacer: bigIntReplacer, ...params }), + defaultSerializeQueryArgs, context.endpointDefinitions[name], name, ) From e22da966a492e5639a2c5f3be14e250105e8d925 Mon Sep 17 00:00:00 2001 From: Roman Kofman <35267+rkofman@users.noreply.github.com> Date: Mon, 8 Apr 2024 10:39:45 -0500 Subject: [PATCH 5/5] add test for bigint in defaultSerializeQueryArgs --- .../src/query/tests/defaultSerializeQueryArgs.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/toolkit/src/query/tests/defaultSerializeQueryArgs.test.ts b/packages/toolkit/src/query/tests/defaultSerializeQueryArgs.test.ts index fa9ad3b72..13b8ae106 100644 --- a/packages/toolkit/src/query/tests/defaultSerializeQueryArgs.test.ts +++ b/packages/toolkit/src/query/tests/defaultSerializeQueryArgs.test.ts @@ -23,6 +23,16 @@ test('number arg', () => { ).toMatchInlineSnapshot(`"test(5)"`) }) +test('bigint arg has non-default serialization (intead of throwing)', () => { + expect( + defaultSerializeQueryArgs({ + endpointDefinition, + endpointName, + queryArgs: BigInt(10), + }), + ).toMatchInlineSnapshot(`"test({"$bigint":"10"})"`) +}) + test('simple object arg is sorted', () => { expect( defaultSerializeQueryArgs({