From eba669ffebe2efef5ed8f9b06a301f0e1fc6d1ef Mon Sep 17 00:00:00 2001
From: Gordon McNaughton <gmcnaughton@hubspot.com>
Date: Wed, 17 Feb 2021 22:27:40 -0500
Subject: [PATCH] Fixes bad intermediate return values from
 `useStoreDependency`

Fixes an issue where `useStoreDependency` would return an incorrect
intermediate value when a prop change causes the dependency value to
change.

Now the updated value is returned immediately when the dependency value
changes, rather than being set in state but not returned until the next
render cycle.

=== Before

The render cycle for a prop change looked like this:

1. component renders: `useStoreDependency` calculates the value, stores
   it in initial state, and returns it

2. prop change causes component to re-render: `useStoreDependency`
   calculates new value; if the value changed from (1): stores the
   new value in state, __but returns the old cached value from (1)__.

3. internal setState call causes component to re-render:
   `useStoreDependency` calculates new value, but the value doesn't
   change (props are same as in (2)): finally returns the new value.

=== After

The render cycle is the same as above, except (2) returns the updated
value immediately instead of the outdated cached value from (1)

Signed-off-by: Gordon McNaughton <gmcnaughton@hubspot.com>
---
 .../__tests__/useStoreDependency-test.js      | 22 +++++++++++++++++++
 src/dependencies/useStoreDependency.ts        |  1 +
 2 files changed, 23 insertions(+)

diff --git a/src/dependencies/__tests__/useStoreDependency-test.js b/src/dependencies/__tests__/useStoreDependency-test.js
index a28630f..a9dd742 100644
--- a/src/dependencies/__tests__/useStoreDependency-test.js
+++ b/src/dependencies/__tests__/useStoreDependency-test.js
@@ -236,6 +236,28 @@ describe('useStoreDependency', () => {
       testHook(useCallback);
     });
 
+    it('returns the updated store value after prop update', () => {
+      const Component = ({ factor }) => {
+        const value = useStoreDependency(multiplyDependency, { factor });
+        if (value !== factor) {
+          // We need to throw here rather than relying on the `expect` below
+          // to catch invalid return values.
+          //
+          // This component ends up re-rendering twice: once for the explicit `setProps`
+          // call, and once for the internal `setState` call inside `useStoreDependency`.
+          // The expected end state is eventually reached, but intermediate return
+          // values from `useStoreDependency` could still be wrong.
+          throw new Error(`incorrect return value from useStoreDependency: expected ${factor} but received ${value}`);
+        }
+        return <div data-value={value} />;
+      };
+      const rendered = mount(<Component factor={4} />);
+      expect(rendered.find('div').prop('data-value')).toBe(4);
+      rendered.setProps({ factor: 8 });
+      rendered.update();
+      expect(rendered.find('div').prop('data-value')).toBe(8);
+    });
+
     it('handles a prop update', () => {
       const Component = ({ factor = 4 }) => {
         const value = useStoreDependency(multiplyDependency, { factor });
diff --git a/src/dependencies/useStoreDependency.ts b/src/dependencies/useStoreDependency.ts
index c26dc9b..8f8c486 100644
--- a/src/dependencies/useStoreDependency.ts
+++ b/src/dependencies/useStoreDependency.ts
@@ -102,6 +102,7 @@ function useStoreDependency<Props, DepType>(
   const newValue = { dependency: calculate(dependency, props) };
   if (!shallowEqual(newValue.dependency, dependencyValue.dependency)) {
     setDependencyValue(newValue);
+    return newValue.dependency;
   }
   return dependencyValue.dependency;
 }