From 51d7899b0da999a48aaf51228f21dd5cdd0c37ae Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Wed, 22 Jun 2022 19:29:31 +0800 Subject: [PATCH] fix(theme-common): add a missing generic constraint (#7648) --- .../src/utils/__tests__/reactUtils.test.ts | 31 ++++++++++++++----- .../src/utils/reactUtils.tsx | 24 ++++++-------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/packages/docusaurus-theme-common/src/utils/__tests__/reactUtils.test.ts b/packages/docusaurus-theme-common/src/utils/__tests__/reactUtils.test.ts index df6e985d01fa..81785b659a97 100644 --- a/packages/docusaurus-theme-common/src/utils/__tests__/reactUtils.test.ts +++ b/packages/docusaurus-theme-common/src/utils/__tests__/reactUtils.test.ts @@ -27,9 +27,12 @@ describe('useShallowMemoObject', () => { const someArray = ['hello', 'world']; const obj1 = {a: 1, b: '2', someObj, someArray}; - const {result, rerender} = renderHook((val) => useShallowMemoObject(val), { - initialProps: obj1, - }); + const {result, rerender} = renderHook( + (val) => useShallowMemoObject(val), + { + initialProps: obj1, + }, + ); expect(result.current).toBe(obj1); const obj2 = {a: 1, b: '2', someObj, someArray}; @@ -40,17 +43,31 @@ describe('useShallowMemoObject', () => { rerender(obj3); expect(result.current).toBe(obj1); - // Current implementation is basic and sensitive to order const obj4 = {b: '2', a: 1, someObj, someArray}; rerender(obj4); - expect(result.current).toBe(obj4); + expect(result.current).toBe(obj1); const obj5 = {b: '2', a: 1, someObj, someArray}; rerender(obj5); - expect(result.current).toBe(obj4); + expect(result.current).toBe(obj1); - const obj6 = {b: '2', a: 1, someObj: {...someObj}, someArray}; + const obj6 = {b: 1, a: '2', someObj, someArray}; rerender(obj6); expect(result.current).toBe(obj6); + expect(result.current).not.toBe(obj5); + + const obj7 = {b: 1, a: '2', someObj: {...someObj}, someArray}; + rerender(obj7); + expect(result.current).toBe(obj7); + expect(result.current).not.toBe(obj6); + + const obj8 = {...obj7}; + rerender(obj8); + expect(result.current).toBe(obj7); + + const obj9 = {...obj7, another: true}; + rerender(obj9); + expect(result.current).toBe(obj9); + expect(result.current).not.toBe(obj7); }); }); diff --git a/packages/docusaurus-theme-common/src/utils/reactUtils.tsx b/packages/docusaurus-theme-common/src/utils/reactUtils.tsx index e03d3c8de609..fea9269b570d 100644 --- a/packages/docusaurus-theme-common/src/utils/reactUtils.tsx +++ b/packages/docusaurus-theme-common/src/utils/reactUtils.tsx @@ -76,21 +76,17 @@ export class ReactContextError extends Error { } /** - * Shallow-memoize an object - * - * This means the returned object will be the same as the previous render - * if the attribute names and identities did not change. - * - * This works for simple cases: when attributes are primitives or stable objects + * Shallow-memoize an object. This means the returned object will be the same as + * the previous render if the property keys and values did not change. This + * works for simple cases: when property values are primitives or stable + * objects. * * @param obj */ -export function useShallowMemoObject(obj: O): O { - return useMemo( - () => obj, - // Is this safe? - // TODO make this implementation not order-dependent? - // eslint-disable-next-line react-hooks/exhaustive-deps - [...Object.keys(obj), ...Object.values(obj)], - ); +export function useShallowMemoObject(obj: O): O { + const deps = Object.entries(obj); + // Sort by keys to make it order-insensitive + deps.sort((a, b) => a[0].localeCompare(b[0])); + // eslint-disable-next-line react-hooks/exhaustive-deps + return useMemo(() => obj, deps.flat()); }