From 8b62c14e9bf2976821fa1991e769c8809a4fe73d Mon Sep 17 00:00:00 2001 From: Josh Fraser Date: Sun, 17 Oct 2021 03:16:59 +1100 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20Fix=20bug=20with=20`useMutat?= =?UTF-8?q?ion`=20shared=20results?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix a scenario where the component calling `reset` needed to call it twice when using shared component results --- .../toolkit/src/query/react/buildHooks.ts | 3 +- .../tests/useMutation-fixedCacheKey.test.tsx | 73 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index f2c4c612f6..110a534b92 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -827,7 +827,8 @@ export function buildHooks({ const reset = useCallback(() => { if (promise) { setPromise(undefined) - } else if (fixedCacheKey) { + } + if (fixedCacheKey) { dispatch( api.internalActions.removeMutationResult({ requestId, diff --git a/packages/toolkit/src/query/tests/useMutation-fixedCacheKey.test.tsx b/packages/toolkit/src/query/tests/useMutation-fixedCacheKey.test.tsx index 1fa0111e7e..c0c5290238 100644 --- a/packages/toolkit/src/query/tests/useMutation-fixedCacheKey.test.tsx +++ b/packages/toolkit/src/query/tests/useMutation-fixedCacheKey.test.tsx @@ -94,6 +94,79 @@ describe('fixedCacheKey', () => { expect(getByTestId(c2, 'data').textContent).toBe('') }) + test('resetting from the component that triggered the mutation resets for each shared result', async () => { + render( + <> + + + + + , + { wrapper: storeRef.wrapper } + ) + const c1 = screen.getByTestId('C1') + const c2 = screen.getByTestId('C2') + const c3 = screen.getByTestId('C3') + const c4 = screen.getByTestId('C4') + expect(getByTestId(c1, 'status').textContent).toBe('uninitialized') + expect(getByTestId(c2, 'status').textContent).toBe('uninitialized') + expect(getByTestId(c3, 'status').textContent).toBe('uninitialized') + expect(getByTestId(c4, 'status').textContent).toBe('uninitialized') + + // trigger with a component using the first cache key + getByTestId(c1, 'trigger').click() + + await waitFor(() => + expect(getByTestId(c1, 'status').textContent).toBe('fulfilled') + ) + + // the components with the first cache key should be affected + expect(getByTestId(c1, 'data').textContent).toBe('C1') + expect(getByTestId(c2, 'status').textContent).toBe('fulfilled') + expect(getByTestId(c2, 'data').textContent).toBe('C1') + expect(getByTestId(c2, 'status').textContent).toBe('fulfilled') + + // the components with the second cache key should be unaffected + expect(getByTestId(c3, 'data').textContent).toBe('') + expect(getByTestId(c3, 'status').textContent).toBe('uninitialized') + expect(getByTestId(c4, 'data').textContent).toBe('') + expect(getByTestId(c4, 'status').textContent).toBe('uninitialized') + + // trigger with a component using the second cache key + getByTestId(c3, 'trigger').click() + + await waitFor(() => + expect(getByTestId(c3, 'status').textContent).toBe('fulfilled') + ) + + // the components with the first cache key should be unaffected + expect(getByTestId(c1, 'data').textContent).toBe('C1') + expect(getByTestId(c2, 'status').textContent).toBe('fulfilled') + expect(getByTestId(c2, 'data').textContent).toBe('C1') + expect(getByTestId(c2, 'status').textContent).toBe('fulfilled') + + // the component with the second cache key should be affected + expect(getByTestId(c3, 'data').textContent).toBe('C3') + expect(getByTestId(c3, 'status').textContent).toBe('fulfilled') + expect(getByTestId(c4, 'data').textContent).toBe('C3') + expect(getByTestId(c4, 'status').textContent).toBe('fulfilled') + + // test reset from the component that triggered the mutation for the first cache key + getByTestId(c1, 'reset').click() + + // the components with the first cache key should be affected + expect(getByTestId(c1, 'data').textContent).toBe('') + expect(getByTestId(c1, 'status').textContent).toBe('uninitialized') + expect(getByTestId(c2, 'data').textContent).toBe('') + expect(getByTestId(c2, 'status').textContent).toBe('uninitialized') + + // the components with the second cache key should be unaffected + expect(getByTestId(c3, 'data').textContent).toBe('C3') + expect(getByTestId(c3, 'status').textContent).toBe('fulfilled') + expect(getByTestId(c4, 'data').textContent).toBe('C3') + expect(getByTestId(c4, 'status').textContent).toBe('fulfilled') + }) + test('two mutations with different `fixedCacheKey` do not influence each other', async () => { render( <> From f2c360631b49b551af9aeff6de8258d53fc294e4 Mon Sep 17 00:00:00 2001 From: Josh Fraser Date: Sun, 17 Oct 2021 07:28:37 +1100 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Make=20`useMutation`?= =?UTF-8?q?=20`reset`=20use=20batched=20updates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../toolkit/src/query/react/buildHooks.ts | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index 110a534b92..07b067b6d0 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -825,17 +825,19 @@ export function buildHooks({ const originalArgs = fixedCacheKey == null ? promise?.arg.originalArgs : undefined const reset = useCallback(() => { - if (promise) { - setPromise(undefined) - } - if (fixedCacheKey) { - dispatch( - api.internalActions.removeMutationResult({ - requestId, - fixedCacheKey, - }) - ) - } + batch(() => { + if (promise) { + setPromise(undefined) + } + if (fixedCacheKey) { + dispatch( + api.internalActions.removeMutationResult({ + requestId, + fixedCacheKey, + }) + ) + } + }) }, [dispatch, fixedCacheKey, promise, requestId]) const finalState = useMemo( () => ({ ...currentState, originalArgs, reset }),