From ceae1ec331bfd61cbb8b99b6cd9dd8d538a22e15 Mon Sep 17 00:00:00 2001 From: Daishi Kato Date: Fri, 12 Jul 2024 09:42:55 +0900 Subject: [PATCH] fix: Static props prevents selectors recall [2] (#102) * add failing test * simplify the test * minor adjustment * fix it, but not super confident * update CHANGELOG --- CHANGELOG.md | 1 + src/memoize.ts | 21 ++++++++++-------- tests/issue_100.spec.ts | 49 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fb5403..ce2de75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Changed - fix: Static props prevents selectors recall #101 +- fix: Static props prevents selectors recall [2] #102 ## [3.0.0] - 2024-05-02 diff --git a/src/memoize.ts b/src/memoize.ts index 55e0ac1..10f8802 100644 --- a/src/memoize.ts +++ b/src/memoize.ts @@ -21,18 +21,18 @@ type Used = { }; type Affected = WeakMap; -const trackMemoOriginalObjSet = new WeakSet(); +const trackMemoUntrackedObjSet = new WeakSet(); const isObject = (x: unknown): x is object => typeof x === 'object' && x !== null; const untrack = (x: T, seen: WeakSet): T => { if (!isObject(x)) return x; - const originalObj = getUntracked(x); - if (originalObj !== null) { + const untrackedObj = getUntracked(x); + if (untrackedObj) { trackMemo(x); - trackMemoOriginalObjSet.add(originalObj); - return originalObj; + trackMemoUntrackedObjSet.add(untrackedObj); + return untrackedObj; } if (!seen.has(x)) { seen.add(x); @@ -46,11 +46,14 @@ const untrack = (x: T, seen: WeakSet): T => { const touchAffected = (dst: unknown, src: unknown, affected: Affected) => { if (!isObject(dst) || !isObject(src)) return; - if (trackMemoOriginalObjSet.has(getUntracked(src) as never)) { - trackMemo(dst); + const untrackedObj = getUntracked(src); + const used = affected.get(untrackedObj || src); + if (!used) { + if (trackMemoUntrackedObjSet.has(untrackedObj as never)) { + trackMemo(dst); + } + return; } - const used = affected.get(getUntracked(src) || src); - if (!used) return; used[HAS_KEY_PROPERTY]?.forEach((key) => { Reflect.has(dst, key); }); diff --git a/tests/issue_100.spec.ts b/tests/issue_100.spec.ts index bb852f0..63691cc 100644 --- a/tests/issue_100.spec.ts +++ b/tests/issue_100.spec.ts @@ -24,7 +24,7 @@ describe('Static props prevents selectors recall (#100)', () => { }, }; - const selectAllBooks = memoize((state: State) => Object.values(state)); + const selectBook1 = memoize((state: State) => state.book1); const selectPriceString = memoize( (state: State) => state.book1.priceString, @@ -36,7 +36,52 @@ describe('Static props prevents selectors recall (#100)', () => { return priceString; }); - selectAllBooks(state1); + selectBook1(state1); + + expect(selectAdjustedPriceString(state1)).toBe('10'); + expect(selectAdjustedPriceString(state2)).toBe('20'); + }); + + it('case 2', () => { + type State = { + book1: { + staticProp: string; + priceString: string; + }; + }; + + const state1: State = { + book1: { + staticProp: '5', + priceString: '10', + }, + }; + + const state2: State = { + book1: { + staticProp: '5', + priceString: '20', + }, + }; + + const selectBook1 = memoize((state: State) => state.book1); + + const selectPriceString = memoize( + (state: State) => state.book1.priceString, + ); + + const selectAdjustedPriceString = memoize((state: State) => { + const priceString = selectPriceString(state); + state.book1.staticProp; // touch the prop + return priceString; + }); + + const selectMemoizedPriceString = memoize((state: State) => + selectPriceString(state), + ); + + selectBook1(state1); + selectMemoizedPriceString(state1); expect(selectAdjustedPriceString(state1)).toBe('10'); expect(selectAdjustedPriceString(state2)).toBe('20');