Skip to content

Commit

Permalink
Fix useMutableSource tearing bug
Browse files Browse the repository at this point in the history
Fix is to move the entanglement call outside of the block that checks
if the snapshot has changed.
  • Loading branch information
acdlite committed May 13, 2020
1 parent 43ce30b commit c3c4c3f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 15 deletions.
11 changes: 4 additions & 7 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -997,14 +997,11 @@ function useMutableSource<Source, Snapshot>(
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]);

Expand Down
15 changes: 7 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -986,15 +986,14 @@ function useMutableSource<Source, Snapshot>(
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]);

Expand Down

0 comments on commit c3c4c3f

Please sign in to comment.