From d963af001342d6d417cc082f060b75b613dbbbc6 Mon Sep 17 00:00:00 2001 From: Gordon McNaughton Date: Thu, 18 Feb 2021 22:12:28 -0500 Subject: [PATCH] Avoids duplicate deref calculations in `useStoreDependency` See https://github.com/HubSpot/general-store/pull/95 Avoids duplicate deref calculations in `useStoreDependency`. Only calculate once; use this result to initialize state (if necessary) and as the next state. When the value changes, two deref calculations will still run: 1. the initial render/hook call 2. the re-render/hook call triggered by setting the new value in state --- .../__tests__/useStoreDependency-test.js | 36 +++++++++++++++++++ src/dependencies/useStoreDependency.ts | 6 ++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/dependencies/__tests__/useStoreDependency-test.js b/src/dependencies/__tests__/useStoreDependency-test.js index a9dd742..125be23 100644 --- a/src/dependencies/__tests__/useStoreDependency-test.js +++ b/src/dependencies/__tests__/useStoreDependency-test.js @@ -214,13 +214,16 @@ describe('useStoreDependency', () => { describe('with props', () => { let store; let multiplyDependency; + let derefCallCount; beforeEach(() => { store = BaseStoreFactory.register(dispatcher); + derefCallCount = 0; multiplyDependency = { stores: [store], deref: ({ factor }) => { if (!factor) throw new Error('no factor provided'); + derefCallCount += 1; return store.get() * factor; }, }; @@ -287,5 +290,38 @@ describe('useStoreDependency', () => { rendered.update(); expect(rendered.find('div').prop('data-factor')).toBe(16); }); + + describe('deref call count', () => { + it('called once on initial render', () => { + const Component = ({ factor }) => { + const value = useStoreDependency(multiplyDependency, { factor }); + return
; + }; + mount(); + expect(derefCallCount).toBe(1); + }); + + it('called once on re-render', () => { + const Component = ({ factor }) => { + const value = useStoreDependency(multiplyDependency, { factor }); + return
; + }; + const rendered = mount(); + rendered.setProps({}); + rendered.update(); + expect(derefCallCount).toBe(2); + }); + + it('called twice on prop change (once from the initial re-render, again in the second re-render after the internal setState call)', () => { + const Component = ({ factor }) => { + const value = useStoreDependency(multiplyDependency, { factor }); + return
; + }; + const rendered = mount(); + rendered.setProps({ factor: 8 }); + rendered.update(); + expect(derefCallCount).toBe(3); + }); + }); }); }); diff --git a/src/dependencies/useStoreDependency.ts b/src/dependencies/useStoreDependency.ts index 8f8c486..2395647 100644 --- a/src/dependencies/useStoreDependency.ts +++ b/src/dependencies/useStoreDependency.ts @@ -83,9 +83,8 @@ function useStoreDependency( ): DepType { enforceDispatcher(dispatcher); - const [dependencyValue, setDependencyValue] = useState({ - dependency: calculate(dependency, props), - }); + const newValue = { dependency: calculate(dependency, props) }; + const [dependencyValue, setDependencyValue] = useState(newValue); const currProps = useCurrent(props); @@ -99,7 +98,6 @@ function useStoreDependency( setDependencyValue ); - const newValue = { dependency: calculate(dependency, props) }; if (!shallowEqual(newValue.dependency, dependencyValue.dependency)) { setDependencyValue(newValue); return newValue.dependency;