Skip to content

Commit

Permalink
Fixed high pri updates erasing pending lower pri subscription updates
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Feb 13, 2020
1 parent 94faec2 commit 3a2a112
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
6 changes: 6 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,12 @@ function useMutableSourceImpl<S>(
isSafeToReadFromSource =
pendingExpirationTime === NoWork ||
pendingExpirationTime >= expirationTime;

if (!isSafeToReadFromSource) {
// Preserve the pending update if the current priority doesn't include it.
currentlyRenderingFiber.expirationTime = pendingExpirationTime;
markUnprocessedUpdateTime(pendingExpirationTime);
}
}

let prevMemoizedState = ((hook.memoizedState: any): ?MutableSourceState<S>);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2088,7 +2088,6 @@ describe('ReactHooksWithNoopRenderer', () => {
() => Scheduler.unstable_yieldValue('Sync effect'),
);
expect(Scheduler).toFlushAndYield(['a:one', 'b:one', 'Sync effect']);
ReactNoop.flushPassiveEffects();

// Changing values should schedule an update with React.
// Start working on this update but don't finish it.
Expand All @@ -2108,9 +2107,7 @@ describe('ReactHooksWithNoopRenderer', () => {
});
expect(Scheduler).toHaveYielded(['a:one', 'b:one', 'Sync effect']);

// TODO (useMutableSource) Re-enable the assertion below; it fails now for reasons unknown.
// Once the update is processed, the new value should be used
// expect(Scheduler).toFlushAndYield(['a:two', 'b:two']);
expect(Scheduler).toFlushAndYield(['a:two', 'b:two']);
});

it('should read from source on newly mounted subtree if no pending updates are scheduled for source', () => {
Expand Down Expand Up @@ -2170,6 +2167,12 @@ describe('ReactHooksWithNoopRenderer', () => {
});
expect(Scheduler).toHaveYielded(['a:two', 'b:two', 'Sync effect']);
});

// TODO (useMutableSource) Test case for a scoped subscription,
// followed by a render that reads from a different part of the store,
// with a mutation between its first and second read,
// that isn't picked up on by the scoped subscription,
// to verify that we also use the version number to protect against this case.
});

describe('useCallback', () => {
Expand Down

0 comments on commit 3a2a112

Please sign in to comment.