From 31a3547a57acd6874e668eb1dd8d0e219a33b611 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Sat, 13 Apr 2024 03:20:34 +0800 Subject: [PATCH] fix(updater): fix muti-updater in react with patches mode (#8) --- src/index.ts | 124 +++++++++-- test/index.test.ts | 499 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 520 insertions(+), 103 deletions(-) diff --git a/src/index.ts b/src/index.ts index d2c6663..ec4dfc8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,7 @@ import { useMemo, useRef, Dispatch, + useEffect, } from 'react'; type PatchesOptions = @@ -37,8 +38,8 @@ type Result = O extends | object ? [F extends true ? Immutable : S, Updater, Patches, Patches] : F extends true - ? [Immutable, Updater] - : [S, Updater]; + ? [Immutable, Updater] + : [S, Updater]; /** * `useMutative` is a hook that is similar to `useState` but it uses `mutative` to handle the state updates. @@ -64,7 +65,7 @@ type Result = O extends function useMutative< S, F extends boolean = false, - O extends PatchesOptions = false + O extends PatchesOptions = false, >( /** * The initial state. You may optionally provide an initializer function to calculate the initial state. @@ -75,21 +76,64 @@ function useMutative< */ options?: Options ) { - const [state, setState] = useState(() => { - const initialState = - typeof initialValue === 'function' ? initialValue() : initialValue; - return options?.enablePatches ? [initialState, [], []] : initialState; + const patchesRef = useRef<{ + patches: Patches; + inversePatches: Patches; + }>({ + patches: [], + inversePatches: [], }); + //#region support strict mode and concurrent features + const count = useRef(0); + const renderCount = useRef(0); + let currentCount = count.current; + useEffect(() => { + count.current = currentCount; + renderCount.current = currentCount; + }); + currentCount += 1; + renderCount.current += 1; + //#endregion + const [state, setState] = useState(() => + typeof initialValue === 'function' ? initialValue() : initialValue + ); const updateState = useCallback((updater: any) => { setState((latest: any) => { - const currentState = options?.enablePatches ? latest[0] : latest; const updaterFn = typeof updater === 'function' ? updater : () => updater; - return create(currentState, updaterFn, options); + const result = create(latest, updaterFn, options); + if (options?.enablePatches) { + // check render count, support strict mode and concurrent features + if ( + renderCount.current === count.current || + renderCount.current === count.current + 1 + ) { + Array.prototype.push.apply(patchesRef.current.patches, result[1]); + // `inversePatches` should be in reverse order when multiple setState() executions + Array.prototype.unshift.apply( + patchesRef.current.inversePatches, + result[2] + ); + } + return result[0]; + } + return result; }); }, []); + useEffect(() => { + if (options?.enablePatches) { + // Reset `patchesRef` when the component is rendered each time + patchesRef.current.patches = []; + patchesRef.current.inversePatches = []; + } + }); return ( options?.enablePatches - ? [state[0], updateState, state[1], state[2]] + ? [ + state, + updateState, + patchesRef.current.patches, + patchesRef.current.inversePatches, + ] : [state, updateState] ) as Result, O, F>; } @@ -98,12 +142,12 @@ type ReducerResult< S, A, O extends PatchesOptions, - F extends boolean + F extends boolean, > = O extends true | object ? [F extends true ? Immutable : S, Dispatch, Patches, Patches] : F extends true - ? [Immutable, Dispatch] - : [S, Dispatch]; + ? [Immutable, Dispatch] + : [S, Dispatch]; type Reducer = (draftState: Draft, action: A) => void | S | undefined; @@ -112,7 +156,7 @@ function useMutativeReducer< A, I, F extends boolean = false, - O extends PatchesOptions = false + O extends PatchesOptions = false, >( reducer: Reducer, initializerArg: S & I, @@ -125,7 +169,7 @@ function useMutativeReducer< A, I, F extends boolean = false, - O extends PatchesOptions = false + O extends PatchesOptions = false, >( reducer: Reducer, initializerArg: I, @@ -137,7 +181,7 @@ function useMutativeReducer< S, A, F extends boolean = false, - O extends PatchesOptions = false + O extends PatchesOptions = false, >( reducer: Reducer, initialState: S, @@ -182,7 +226,7 @@ function useMutativeReducer< A, I, F extends boolean = false, - O extends PatchesOptions = false + O extends PatchesOptions = false, >( /** * A function that returns the next state tree, given the current state tree and the action to handle. @@ -201,7 +245,24 @@ function useMutativeReducer< */ options?: Options ): ReducerResult { - const ref = useRef<[Patches, Patches]>([[], []]); + const patchesRef = useRef<{ + patches: Patches; + inversePatches: Patches; + }>({ + patches: [], + inversePatches: [], + }); + //#region support strict mode and concurrent features + const count = useRef(0); + const renderCount = useRef(0); + let currentCount = count.current; + useEffect(() => { + count.current = currentCount; + renderCount.current = currentCount; + }); + currentCount += 1; + renderCount.current += 1; + //#endregion const cachedReducer: any = useMemo( () => (state: any, action: any) => { const result: any = create( @@ -210,7 +271,18 @@ function useMutativeReducer< options ); if (options?.enablePatches) { - ref.current = [result[1], result[2]]; + // check render count, support strict mode and concurrent features + if ( + renderCount.current === count.current || + renderCount.current === count.current + 1 + ) { + Array.prototype.push.apply(patchesRef.current.patches, result[1]); + // `inversePatches` should be in reverse order when multiple setState() executions + Array.prototype.unshift.apply( + patchesRef.current.inversePatches, + result[2] + ); + } return result[0]; } return result; @@ -222,8 +294,20 @@ function useMutativeReducer< initializerArg as any, initializer as any ); + useEffect(() => { + if (options?.enablePatches) { + // Reset `patchesRef` when the component is rendered each time + patchesRef.current.patches = []; + patchesRef.current.inversePatches = []; + } + }); return options?.enablePatches - ? [result[0], result[1], ref.current[0], ref.current[1]] + ? [ + result[0], + result[1], + patchesRef.current.patches, + patchesRef.current.inversePatches, + ] : result; } diff --git a/test/index.test.ts b/test/index.test.ts index 2fa53cd..75d4b01 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -1,7 +1,7 @@ import { jsdocTests } from 'jsdoc-tests'; import { act, renderHook } from '@testing-library/react'; -import { rawReturn, type Draft } from 'mutative'; -import React from 'react'; +import { rawReturn, type Draft, apply } from 'mutative'; +import React, { useState } from 'react'; import { useMutative, useMutativeReducer } from '../src/index'; @@ -138,6 +138,312 @@ describe('useMutative', () => { const [state3] = result.current; expect(state3).toEqual({ items: [123] }); }); + + it('[useMutative] with patches and multiple updates', () => { + const baseState = [1]; + const { result } = renderHook(() => + useMutative(() => baseState, { enablePatches: true }) + ); + + expect(result.current).toHaveLength(4); + + let [state, setState, patches, inversePatches] = result.current; + expect(state).toEqual([1]); + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + act(() => { + setState((draft) => { + draft.push(2); + draft.push(999); + }); + setState((draft) => { + draft.push(3); + draft.push(4); + }); + setState((draft) => { + draft.shift(); + draft.push(5); + }); + }); + + [state, setState, patches, inversePatches] = result.current; + const mutateState = () => { + const state = [1]; + state.push(2); + state.push(999); + // + state.push(3); + state.push(4); + // + state.shift(); + state.push(5); + return state; + }; + expect(baseState).toEqual([1]); + expect(state).toEqual([2, 999, 3, 4, 5]); + expect(mutateState()).toEqual([2, 999, 3, 4, 5]); + expect(patches).toEqual([ + { op: 'add', path: [1], value: 2 }, + { op: 'add', path: [2], value: 999 }, + { op: 'add', path: [3], value: 3 }, + { op: 'add', path: [4], value: 4 }, + { op: 'replace', path: [0], value: 2 }, + { op: 'replace', path: [1], value: 999 }, + { op: 'replace', path: [2], value: 3 }, + { op: 'replace', path: [3], value: 4 }, + { op: 'replace', path: [4], value: 5 }, + ]); + expect(inversePatches).toEqual([ + { op: 'replace', path: [0], value: 1 }, + { op: 'replace', path: [1], value: 2 }, + { op: 'replace', path: [2], value: 999 }, + { op: 'replace', path: [3], value: 3 }, + { op: 'replace', path: [4], value: 4 }, + { op: 'replace', path: ['length'], value: 3 }, + { op: 'replace', path: ['length'], value: 1 }, + ]); + expect(apply(baseState, patches)).toEqual(state); + expect(apply(state, inversePatches)).toEqual(baseState); + }); + + it('[useMutative] with patches and multiple updates, other state ', () => { + const baseState = [1]; + const { result } = renderHook(() => { + return [ + useMutative(() => baseState, { enablePatches: true }) as any, + useState(0), + ]; + }); + + let [[state, setState, patches, inversePatches], [state0, setState0]] = + result.current; + expect(state0).toEqual(0); + expect(state).toEqual([1]); + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + act(() => { + setState0(1); + }); + + [[state, setState, patches, inversePatches], [state0, setState0]] = + result.current; + expect(state0).toEqual(1); + expect(state).toEqual([1]); + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + act(() => { + setState((draft: any) => { + draft.push(2); + draft.push(999); + }); + setState((draft: any) => { + draft.push(3); + draft.push(4); + }); + setState((draft: any) => { + draft.shift(); + draft.push(5); + }); + }); + + [[state, setState, patches, inversePatches], [state0, setState0]] = + result.current; + const mutateState = () => { + const state = [1]; + state.push(2); + state.push(999); + // + state.push(3); + state.push(4); + // + state.shift(); + state.push(5); + return state; + }; + expect(baseState).toEqual([1]); + expect(state).toEqual([2, 999, 3, 4, 5]); + expect(mutateState()).toEqual([2, 999, 3, 4, 5]); + expect(patches).toEqual([ + { op: 'add', path: [1], value: 2 }, + { op: 'add', path: [2], value: 999 }, + { op: 'add', path: [3], value: 3 }, + { op: 'add', path: [4], value: 4 }, + { op: 'replace', path: [0], value: 2 }, + { op: 'replace', path: [1], value: 999 }, + { op: 'replace', path: [2], value: 3 }, + { op: 'replace', path: [3], value: 4 }, + { op: 'replace', path: [4], value: 5 }, + ]); + expect(inversePatches).toEqual([ + { op: 'replace', path: [0], value: 1 }, + { op: 'replace', path: [1], value: 2 }, + { op: 'replace', path: [2], value: 999 }, + { op: 'replace', path: [3], value: 3 }, + { op: 'replace', path: [4], value: 4 }, + { op: 'replace', path: ['length'], value: 3 }, + { op: 'replace', path: ['length'], value: 1 }, + ]); + expect(apply(baseState, patches)).toEqual(state); + expect(apply(state, inversePatches)).toEqual(baseState); + }); + + it('[useMutative] with patches and multiple updates in StrictMode', () => { + const baseState = [1]; + const { result } = renderHook( + () => useMutative(() => baseState, { enablePatches: true }), + { wrapper: React.StrictMode } + ); + + expect(result.current).toHaveLength(4); + + let [state, setState, patches, inversePatches] = result.current; + expect(state).toEqual([1]); + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + act(() => { + setState((draft) => { + draft.push(2); + draft.push(999); + }); + setState((draft) => { + draft.push(3); + draft.push(4); + }); + setState((draft) => { + draft.shift(); + draft.push(5); + }); + }); + + [state, setState, patches, inversePatches] = result.current; + const mutateState = () => { + const state = [1]; + state.push(2); + state.push(999); + // + state.push(3); + state.push(4); + // + state.shift(); + state.push(5); + return state; + }; + expect(baseState).toEqual([1]); + expect(state).toEqual([2, 999, 3, 4, 5]); + expect(mutateState()).toEqual([2, 999, 3, 4, 5]); + expect(patches).toEqual([ + { op: 'add', path: [1], value: 2 }, + { op: 'add', path: [2], value: 999 }, + { op: 'add', path: [3], value: 3 }, + { op: 'add', path: [4], value: 4 }, + { op: 'replace', path: [0], value: 2 }, + { op: 'replace', path: [1], value: 999 }, + { op: 'replace', path: [2], value: 3 }, + { op: 'replace', path: [3], value: 4 }, + { op: 'replace', path: [4], value: 5 }, + ]); + expect(inversePatches).toEqual([ + { op: 'replace', path: [0], value: 1 }, + { op: 'replace', path: [1], value: 2 }, + { op: 'replace', path: [2], value: 999 }, + { op: 'replace', path: [3], value: 3 }, + { op: 'replace', path: [4], value: 4 }, + { op: 'replace', path: ['length'], value: 3 }, + { op: 'replace', path: ['length'], value: 1 }, + ]); + expect(apply(baseState, patches)).toEqual(state); + expect(apply(state, inversePatches)).toEqual(baseState); + }); + + it('[useMutative] with patches and multiple updates, other state in StrictMode', () => { + const baseState = [1]; + const { result } = renderHook( + () => { + return [ + useMutative(() => baseState, { enablePatches: true }) as any, + useState(0), + ]; + }, + { wrapper: React.StrictMode } + ); + + let [[state, setState, patches, inversePatches], [state0, setState0]] = + result.current; + expect(state0).toEqual(0); + expect(state).toEqual([1]); + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + act(() => { + setState0(1); + }); + + [[state, setState, patches, inversePatches], [state0, setState0]] = + result.current; + expect(state0).toEqual(1); + expect(state).toEqual([1]); + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + act(() => { + setState((draft: any) => { + draft.push(2); + draft.push(999); + }); + setState((draft: any) => { + draft.push(3); + draft.push(4); + }); + setState((draft: any) => { + draft.shift(); + draft.push(5); + }); + }); + + [[state, setState, patches, inversePatches], [state0, setState0]] = + result.current; + const mutateState = () => { + const state = [1]; + state.push(2); + state.push(999); + // + state.push(3); + state.push(4); + // + state.shift(); + state.push(5); + return state; + }; + expect(baseState).toEqual([1]); + expect(state).toEqual([2, 999, 3, 4, 5]); + expect(mutateState()).toEqual([2, 999, 3, 4, 5]); + expect(patches).toEqual([ + { op: 'add', path: [1], value: 2 }, + { op: 'add', path: [2], value: 999 }, + { op: 'add', path: [3], value: 3 }, + { op: 'add', path: [4], value: 4 }, + { op: 'replace', path: [0], value: 2 }, + { op: 'replace', path: [1], value: 999 }, + { op: 'replace', path: [2], value: 3 }, + { op: 'replace', path: [3], value: 4 }, + { op: 'replace', path: [4], value: 5 }, + ]); + expect(inversePatches).toEqual([ + { op: 'replace', path: [0], value: 1 }, + { op: 'replace', path: [1], value: 2 }, + { op: 'replace', path: [2], value: 999 }, + { op: 'replace', path: [3], value: 3 }, + { op: 'replace', path: [4], value: 4 }, + { op: 'replace', path: ['length'], value: 3 }, + { op: 'replace', path: ['length'], value: 1 }, + ]); + expect(apply(baseState, patches)).toEqual(state); + expect(apply(state, inversePatches)).toEqual(baseState); + }); }); describe('useMutativeReducer', () => { @@ -216,6 +522,56 @@ describe('useMutativeReducer', () => { }); it('[useMutativeReducer] with patches', () => { + const { result } = renderHook(() => + useMutativeReducer( + reducer, + initState, + (state) => { + state.title = 'changed title '; + return state; + }, + { enablePatches: true } + ) + ); + + expect(result.current).toHaveLength(4); + + const [state, dispatch, patches, inversePatches] = result.current; + expect(state).toEqual({ + title: 'changed title ', + cars: [{ text: '🚘' }], + }); + + expect(patches).toEqual([]); + expect(inversePatches).toEqual([]); + + expect(typeof dispatch).toBe('function'); + + act(() => { + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + }); + + const [state1, dispatch1, patches1, inversePatches1] = result.current; + expect(state1).toEqual({ + title: 'changed title ll', + cars: [{ text: '🚘' }, { text: '🚘' }, { text: '🚘' }], + }); + expect(apply(state, patches1)).toEqual(state1); + expect(apply(state1, inversePatches1)).toEqual(state); + + act(() => dispatch1({ type: 'reset' })); + + const [state2, dispatch2, patches2, inversePatches2] = result.current; + expect(state2).toEqual({ + title: 'changed title ', + cars: [{ text: '🚘' }], + }); + expect(apply(state1, patches2)).toEqual(state2); + expect(apply(state2, inversePatches2)).toEqual(state1); + }); + + it('[useMutativeReducer] with patches in StrictMode', () => { const { result } = renderHook( () => useMutativeReducer( @@ -243,22 +599,18 @@ describe('useMutativeReducer', () => { expect(typeof dispatch).toBe('function'); - act(() => dispatch({ type: 'increment' })); + act(() => { + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + }); const [state1, dispatch1, patches1, inversePatches1] = result.current; expect(state1).toEqual({ - title: 'changed title l', - cars: [{ text: '🚘' }, { text: '🚘' }], + title: 'changed title ll', + cars: [{ text: '🚘' }, { text: '🚘' }, { text: '🚘' }], }); - - expect(patches1).toEqual([ - { op: 'add', path: ['cars', 1], value: { text: '🚘' } }, - { op: 'replace', path: ['title'], value: 'changed title l' }, - ]); - expect(inversePatches1).toEqual([ - { op: 'replace', path: ['cars', 'length'], value: 1 }, - { op: 'replace', path: ['title'], value: 'changed title ' }, - ]); + expect(apply(state, patches1)).toEqual(state1); + expect(apply(state1, inversePatches1)).toEqual(state); act(() => dispatch1({ type: 'reset' })); @@ -267,24 +619,8 @@ describe('useMutativeReducer', () => { title: 'changed title ', cars: [{ text: '🚘' }], }); - - expect(patches2).toEqual([ - { - op: 'replace', - path: [], - value: { title: 'changed title ', cars: [{ text: '🚘' }] }, - }, - ]); - expect(inversePatches2).toEqual([ - { - op: 'replace', - path: [], - value: { - title: 'changed title l', - cars: [{ text: '🚘' }, { text: '🚘' }], - }, - }, - ]); + expect(apply(state1, patches2)).toEqual(state2); + expect(apply(state2, inversePatches2)).toEqual(state1); }); it('[useMutativeReducer] show warning when change `enablePatches` dynamically', () => { @@ -319,58 +655,55 @@ describe('useMutativeReducer', () => { dispatch({ type: 'increment' }); }); + const copyInitState = JSON.parse(JSON.stringify(initState)); + const mutateState = () => { + const dispatch = (action: any) => reducer(copyInitState, action); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + return copyInitState; + }; + const [state, , patches, inversePatches] = result.current; + expect(state).toEqual(mutateState()); + expect(apply(initState, patches)).toEqual(state); + expect(apply(state, inversePatches)).toEqual(initState); + }); - expect([state, patches, inversePatches]).toEqual([ - { - title: 'changed title lllll', - cars: [ - { - text: '🚘', - }, - { - text: '🚘', - }, - { - text: '🚘', - }, - { - text: '🚘', - }, - { - text: '🚘', - }, - { - text: '🚘', - }, - ], - }, - [ - { - op: 'add', - path: ['cars', 5], - value: { - text: '🚘', - }, - }, - { - op: 'replace', - path: ['title'], - value: 'changed title lllll', - }, - ], - [ - { - op: 'replace', - path: ['cars', 'length'], - value: 5, - }, - { - op: 'replace', - path: ['title'], - value: 'changed title llll', - }, - ], - ]); + it('[useMutativeReducer] show dispatch many times in one tick and StrictMode', () => { + const { result } = renderHook( + () => + useMutativeReducer(reducer, initState, undefined, { + enablePatches: true, + }), + { wrapper: React.StrictMode } + ); + const [, dispatch] = result.current; + + act(() => { + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + }); + + const copyInitState = JSON.parse(JSON.stringify(initState)); + const mutateState = () => { + const dispatch = (action: any) => reducer(copyInitState, action); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + dispatch({ type: 'increment' }); + return copyInitState; + }; + + const [state, , patches, inversePatches] = result.current; + expect(state).toEqual(mutateState()); + expect(apply(initState, patches)).toEqual(state); + expect(apply(state, inversePatches)).toEqual(initState); }); });