From 7c9181fc6cbf1b6541b5ca34c900d59723bf392e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 2 Apr 2020 15:21:53 -0700 Subject: [PATCH] Fixed remaining tearing cases --- .../react-reconciler/src/ReactFiberHooks.js | 23 ++++++++++++++++--- .../src/ReactMutableSource.js | 23 +++++++++---------- .../useMutableSource-test.internal.js | 11 +-------- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index db0da30e7d2f01..a37641245d0e06 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -76,7 +76,6 @@ import { } from './SchedulerWithReactIntegration'; import { getFirstPendingExpirationTime, - getLastPendingExpirationTime, getWorkInProgressVersion, markSourceAsDirty, setPendingExpirationTime, @@ -1026,13 +1025,22 @@ function useMutableSource( if (!is(snapshot, maybeNewSnapshot)) { setSnapshot(maybeNewSnapshot); + const currentTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const expirationTime = computeExpirationForFiber( + currentTime, + fiber, + suspenseConfig, + ); + setPendingExpirationTime(root, expirationTime); + // If the source mutated between render and now, // there may be state updates already scheduled from the old getSnapshot. // Those updates should not commit without this value. // There is no mechanism currently to associate these updates though, // so for now we fall back to synchronously flushing all pending updates. // TODO: Improve this later. - markRootExpiredAtTime(root, getLastPendingExpirationTime(root)); + markRootExpiredAtTime(root, getFirstPendingExpirationTime(root)); } } } @@ -1088,10 +1096,19 @@ function useMutableSource( if (!is(snapshot, maybeNewSnapshot)) { setSnapshot(maybeNewSnapshot); + const currentTime = requestCurrentTimeForUpdate(); + const suspenseConfig = requestCurrentSuspenseConfig(); + const expirationTime = computeExpirationForFiber( + currentTime, + fiber, + suspenseConfig, + ); + setPendingExpirationTime(root, expirationTime); + // We missed a mutation before committing. // It's possible that other components using this source also have pending updates scheduled. // In that case, we should ensure they all commit together. - markRootExpiredAtTime(root, getLastPendingExpirationTime(root)); + markRootExpiredAtTime(root, getFirstPendingExpirationTime(root)); } } diff --git a/packages/react-reconciler/src/ReactMutableSource.js b/packages/react-reconciler/src/ReactMutableSource.js index 99a2c612f4256b..ead89e52f37f44 100644 --- a/packages/react-reconciler/src/ReactMutableSource.js +++ b/packages/react-reconciler/src/ReactMutableSource.js @@ -30,11 +30,14 @@ export function clearPendingUpdates( root: FiberRoot, expirationTime: ExpirationTime, ): void { - if (root.mutableSourceFirstPendingUpdateTime <= expirationTime) { + if (expirationTime <= root.mutableSourceLastPendingUpdateTime) { + // All updates for this source have been processed. root.mutableSourceFirstPendingUpdateTime = NoWork; - } - if (root.mutableSourceLastPendingUpdateTime <= expirationTime) { root.mutableSourceLastPendingUpdateTime = NoWork; + } else if (expirationTime <= root.mutableSourceFirstPendingUpdateTime) { + // The highest priority pending updates have been processed, + // but we don't how many updates remain between the current and lowest priority. + root.mutableSourceFirstPendingUpdateTime = expirationTime - 1; } } @@ -53,18 +56,14 @@ export function setPendingExpirationTime( // Because we only track one pending update time per root, // track the lowest priority update. // It's inclusive of all other pending updates. - root.mutableSourceLastPendingUpdateTime = Math.max( - root.mutableSourceLastPendingUpdateTime, - expirationTime, - ); + if (expirationTime > root.mutableSourceLastPendingUpdateTime) { + root.mutableSourceLastPendingUpdateTime = expirationTime; + } if (root.mutableSourceFirstPendingUpdateTime === NoWork) { root.mutableSourceFirstPendingUpdateTime = expirationTime; - } else { - root.mutableSourceFirstPendingUpdateTime = Math.min( - root.mutableSourceFirstPendingUpdateTime, - expirationTime, - ); + } else if (expirationTime < root.mutableSourceFirstPendingUpdateTime) { + root.mutableSourceFirstPendingUpdateTime = expirationTime; } } diff --git a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js index 114dbb19a000f5..61d753ddf1c99c 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js +++ b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js @@ -1385,9 +1385,7 @@ describe('useMutableSource', () => { }); expect(Scheduler).toFlushUntilNextPaint([]); - // TODO: This test currently tears. - // Fill in with correct values once the entanglement issue has been fixed. - expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a0'); + expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a1'); }); expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a1'); @@ -1481,9 +1479,6 @@ describe('useMutableSource', () => { ); }); - // TODO: This test currently tears. - // Fill in with correct values once the entanglement issue has been fixed. - // Here's the current behavior: expect(Scheduler).toHaveYielded([ // The partial render completes 'Child: 2', @@ -1491,10 +1486,6 @@ describe('useMutableSource', () => { // Then we start rendering the low priority mutation 'Parent: 3', - // But the child never received a mutation event, because it hadn't - // mounted yet. So the render tears. - 'Child: 2', - 'Commit: Oops, tearing!', // Eventually the child corrects itself, because of the check that // occurs when re-subscribing.