From 14c76231d51f13fff2168981cd7befbaf5a60539 Mon Sep 17 00:00:00 2001 From: Huarch Date: Thu, 9 Jul 2026 14:50:33 +0800 Subject: [PATCH] fix(map): prevent controllable state loops --- .../olmap/core/useControllableState.test.ts | 42 +++++++++++++++++++ .../olmap/core/useControllableState.ts | 23 +++++++--- 2 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 src/components/olmap/core/useControllableState.test.ts diff --git a/src/components/olmap/core/useControllableState.test.ts b/src/components/olmap/core/useControllableState.test.ts new file mode 100644 index 0000000..1551882 --- /dev/null +++ b/src/components/olmap/core/useControllableState.test.ts @@ -0,0 +1,42 @@ +import { act, renderHook } from "@testing-library/react"; + +import { useControllableObjectState } from "./useControllableState"; + +describe("useControllableObjectState", () => { + it("keeps controlled setters stable when the external value changes", () => { + const onChange = jest.fn(); + const { result, rerender } = renderHook( + ({ value }: { value: { count: number } }) => + useControllableObjectState(value, onChange, { count: 0 }), + { + initialProps: { value: { count: 0 } }, + }, + ); + + const initialSetValue = result.current[1]; + const initialSetField = result.current[2]; + + act(() => { + result.current[2]("count", 1); + }); + expect(onChange).toHaveBeenCalledWith({ count: 1 }); + + rerender({ value: { count: 1 } }); + + expect(result.current[1]).toBe(initialSetValue); + expect(result.current[2]).toBe(initialSetField); + }); + + it("does not publish unchanged object fields", () => { + const onChange = jest.fn(); + const { result } = renderHook(() => + useControllableObjectState({ count: 1 }, onChange, { count: 0 }), + ); + + act(() => { + result.current[2]("count", 1); + }); + + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/olmap/core/useControllableState.ts b/src/components/olmap/core/useControllableState.ts index 729a849..6abeb6f 100644 --- a/src/components/olmap/core/useControllableState.ts +++ b/src/components/olmap/core/useControllableState.ts @@ -10,10 +10,14 @@ export const useControllableState = ( const [internalValue, setInternalValue] = useState(defaultValue); const value = externalValue !== undefined ? externalValue : internalValue; const valueRef = useRef(value); + const isControlledRef = useRef(externalValue !== undefined); + const onExternalChangeRef = useRef(onExternalChange); useEffect(() => { valueRef.current = value; - }, [value]); + isControlledRef.current = externalValue !== undefined; + onExternalChangeRef.current = onExternalChange; + }, [externalValue, onExternalChange, value]); const setValue = useCallback( (next: T | ((previous: T) => T)) => { @@ -21,13 +25,18 @@ export const useControllableState = ( typeof next === "function" ? (next as (previous: T) => T)(valueRef.current) : next; + + if (Object.is(valueRef.current, nextValue)) { + return; + } + valueRef.current = nextValue; - if (externalValue === undefined) { + if (!isControlledRef.current) { setInternalValue(nextValue); } - onExternalChange?.(nextValue); + onExternalChangeRef.current?.(nextValue); }, - [externalValue, onExternalChange], + [], ); return [value, setValue] as const; @@ -46,7 +55,11 @@ export const useControllableObjectState = ( const setField = useCallback( (key: K, nextValue: T[K]) => { - setValue((previous) => ({ ...previous, [key]: nextValue })); + setValue((previous) => + Object.is(previous[key], nextValue) + ? previous + : { ...previous, [key]: nextValue }, + ); }, [setValue], );