fix: computed props w/ dependencies on store state (#874)

ctrlplusb · Oct 13, 2023 · 282fa1c · 282fa1c
1 parent 11903f8
commit 282fa1c
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 8 deletions.
diff --git 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
@@ -150,15 +150,19 @@ export default function extractDataFromModel(
         } else if (value[computedSymbol]) {
           const parent = get(parentPath, _dS);
           const bindComputedProperty = createComputedPropertyBinder(
-            parentPath,
             key,
             value,
             _r,
           );
           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
@@ -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
+});