From c3c4c3f6f9d8e7ce3268a37828fb8343f5d3dad0 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 13 May 2020 11:32:02 -0700 Subject: [PATCH] Fix useMutableSource tearing bug Fix is to move the entanglement call outside of the block that checks if the snapshot has changed. --- .../react-reconciler/src/ReactFiberHooks.new.js | 11 ++++------- .../react-reconciler/src/ReactFiberHooks.old.js | 15 +++++++-------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 18cbb4f58c428..e2d5244746347 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -997,14 +997,11 @@ function useMutableSource( const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(fiber, suspenseConfig); markRootMutableRead(root, lane); - - // If the source mutated between render and now, - // there may be state updates already scheduled from the old source. - // Entangle the updates so that they render in the same batch. - // TODO: I think we need to entangle even if the snapshot matches, - // because there could have been an update to a different hook. - markRootEntangled(root, root.mutableReadLanes); } + // If the source mutated between render and now, + // there may be state updates already scheduled from the old source. + // Entangle the updates so that they render in the same batch. + markRootEntangled(root, root.mutableReadLanes); } }, [getSnapshot, source, subscribe]); diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index d3b83165ea1b2..fc925dbe7bda2 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -986,15 +986,14 @@ function useMutableSource( 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)); } + // 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)); } }, [getSnapshot, source, subscribe]);