From 282fa1cd2cb953893bce8d12ef3da4ea2c804980 Mon Sep 17 00:00:00 2001 From: Peter Weinberg Date: Fri, 13 Oct 2023 12:00:52 -0400 Subject: [PATCH] fix: computed props w/ dependencies on store state (#874) --- src/computed-properties.js | 13 +++++---- src/extract-data-from-model.js | 8 +++-- tests/computed.test.js | 53 ++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/computed-properties.js b/src/computed-properties.js index 4d0fd052..2b739a8a 100644 --- a/src/computed-properties.js +++ b/src/computed-properties.js @@ -1,8 +1,8 @@ import equal from 'fast-deep-equal/es6'; import { areInputsEqual } from './lib'; -export function createComputedPropertyBinder(parentPath, key, def, _r) { - let runOnce = false; +export function createComputedPropertyBinder(key, def, _r) { + let hasRunOnce = false; let prevInputs = []; let prevValue; let prevStoreState; @@ -28,10 +28,11 @@ export function createComputedPropertyBinder(parentPath, key, def, _r) { const inputs = def.stateResolvers.map((resolver) => resolver(parentState, storeState), ); + if ( - runOnce && - (storeState === prevStoreState || - areInputsEqual(inputs, prevInputs) || + hasRunOnce && + ((storeState === prevStoreState && + areInputsEqual(inputs, prevInputs)) || // We don't want computed properties resolved every time an action // is handled by the reducer. They need to remain lazy, only being // computed when used by a component or getState call; @@ -49,7 +50,7 @@ export function createComputedPropertyBinder(parentPath, key, def, _r) { prevInputs = inputs; prevStoreState = storeState; - runOnce = true; + hasRunOnce = true; return prevValue; }, }); diff --git a/src/extract-data-from-model.js b/src/extract-data-from-model.js index 724710e9..065b180b 100644 --- a/src/extract-data-from-model.js +++ b/src/extract-data-from-model.js @@ -150,7 +150,6 @@ export default function extractDataFromModel( } else if (value[computedSymbol]) { const parent = get(parentPath, _dS); const bindComputedProperty = createComputedPropertyBinder( - parentPath, key, value, _r, @@ -158,7 +157,12 @@ export default function extractDataFromModel( bindComputedProperty(parent, _dS); _cP.push({ key, parentPath, bindComputedProperty }); } else if (value[reducerSymbol]) { - _cR.push({ key, parentPath, reducer: value.fn, config: value.config }); + _cR.push({ + key, + parentPath, + reducer: value.fn, + config: value.config, + }); } else if (value[effectOnSymbol]) { const def = { ...value }; diff --git a/tests/computed.test.js b/tests/computed.test.js index 4d18267a..2d4d69b7 100644 --- a/tests/computed.test.js +++ b/tests/computed.test.js @@ -619,3 +619,56 @@ test('computed properties operate against their original store state', () => { expect(stateAtAPointInTime.count).toBe(1); expect(store.getState().count).toBe(2); }); + +test('issue #873: computed properties without state resolvers update when they have dependencies that depend on store state', () => { + // ARRANGE + const store = createStore({ + foo: { + status: 'Ok', + setStatus: action((state, payload) => { + state.status = payload; + }) + }, + bar: { + status: 'Ok', + setStatus: action((state, payload) => { + state.status = payload; + }) + }, + baz: { + errorMessage: computed( + [ + (_, storeState) => storeState.foo.status, + (_, storeState) => storeState.bar.status + ], + (fooStatus, barStatus) => { + if (fooStatus === 'Error' || barStatus === 'Error') { + return 'Uh oh, error in app!'; + } + + return ''; + } + ), + hasError: computed((state) => Boolean(state.errorMessage)) + } + }); + + let state = store.getState(); + + // ASSERT + expect(state.foo.status).toBe('Ok'); + expect(state.bar.status).toBe('Ok'); + expect(state.baz.errorMessage).toBe(''); + expect(state.baz.hasError).toBe(false); + + // ACT + store.getActions().foo.setStatus('Error'); + + state = store.getState(); + + // ASSERT + expect(state.foo.status).toBe('Error'); + expect(state.bar.status).toBe('Ok'); + expect(state.baz.errorMessage).toBe('Uh oh, error in app!'); + expect(state.baz.hasError).toBe(true); // would have failed before #874 +});