Skip to content

Commit

Permalink
Fixes bad intermediate return values from useStoreDependency
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
gmcnaughton committed Feb 18, 2021
1 parent e7963fe commit eba669f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
22 changes: 22 additions & 0 deletions src/dependencies/__tests__/useStoreDependency-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
1 change: 1 addition & 0 deletions src/dependencies/useStoreDependency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit eba669f

Please sign in to comment.