diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index c85be46427cff2..13f843ad80607c 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -12,51 +12,53 @@ import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; export type Flags = number; // Don't change these two values. They're used by React Dev Tools. -export const NoFlags = /* */ 0b00000000000000000000000; -export const PerformedWork = /* */ 0b00000000000000000000001; +export const NoFlags = /* */ 0b000000000000000000000000; +export const PerformedWork = /* */ 0b000000000000000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b00000000000000000000010; -export const Update = /* */ 0b00000000000000000000100; +export const Placement = /* */ 0b000000000000000000000010; +export const Update = /* */ 0b000000000000000000000100; export const PlacementAndUpdate = /* */ Placement | Update; -export const Deletion = /* */ 0b00000000000000000001000; -export const ChildDeletion = /* */ 0b00000000000000000010000; -export const ContentReset = /* */ 0b00000000000000000100000; -export const Callback = /* */ 0b00000000000000001000000; -export const DidCapture = /* */ 0b00000000000000010000000; -export const Ref = /* */ 0b00000000000000100000000; -export const Snapshot = /* */ 0b00000000000001000000000; -export const Passive = /* */ 0b00000000000010000000000; -export const Hydrating = /* */ 0b00000000000100000000000; +export const Deletion = /* */ 0b000000000000000000001000; +export const ChildDeletion = /* */ 0b000000000000000000010000; +export const ContentReset = /* */ 0b000000000000000000100000; +export const Callback = /* */ 0b000000000000000001000000; +export const DidCapture = /* */ 0b000000000000000010000000; +export const Ref = /* */ 0b000000000000000100000000; +export const Snapshot = /* */ 0b000000000000001000000000; +export const Passive = /* */ 0b000000000000010000000000; +export const Hydrating = /* */ 0b000000000000100000000000; export const HydratingAndUpdate = /* */ Hydrating | Update; -export const Visibility = /* */ 0b00000000001000000000000; +export const Visibility = /* */ 0b000000000001000000000000; +export const StoreConsistency = /* */ 0b000000000010000000000000; -export const LifecycleEffectMask = Passive | Update | Callback | Ref | Snapshot; +export const LifecycleEffectMask = + Passive | Update | Callback | Ref | Snapshot | StoreConsistency; // Union of all commit flags (flags with the lifetime of a particular commit) -export const HostEffectMask = /* */ 0b00000000001111111111111; +export const HostEffectMask = /* */ 0b000000000011111111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b00000000010000000000000; -export const ShouldCapture = /* */ 0b00000000100000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b00000001000000000000000; -export const DidPropagateContext = /* */ 0b00000010000000000000000; -export const NeedsPropagation = /* */ 0b00000100000000000000000; +export const Incomplete = /* */ 0b000000000100000000000000; +export const ShouldCapture = /* */ 0b000000001000000000000000; +export const ForceUpdateForLegacySuspense = /* */ 0b000000010000000000000000; +export const DidPropagateContext = /* */ 0b000000100000000000000000; +export const NeedsPropagation = /* */ 0b000001000000000000000000; // Static tags describe aspects of a fiber that are not specific to a render, // e.g. a fiber uses a passive effect (even if there are no updates on this particular render). // This enables us to defer more work in the unmount case, // since we can defer traversing the tree during layout to look for Passive effects, // and instead rely on the static flag as a signal that there may be cleanup work. -export const RefStatic = /* */ 0b00001000000000000000000; -export const LayoutStatic = /* */ 0b00010000000000000000000; -export const PassiveStatic = /* */ 0b00100000000000000000000; +export const RefStatic = /* */ 0b000010000000000000000000; +export const LayoutStatic = /* */ 0b000100000000000000000000; +export const PassiveStatic = /* */ 0b001000000000000000000000; // These flags allow us to traverse to fibers that have effects on mount // without traversing the entire tree after every commit for // double invoking -export const MountLayoutDev = /* */ 0b01000000000000000000000; -export const MountPassiveDev = /* */ 0b10000000000000000000000; +export const MountLayoutDev = /* */ 0b010000000000000000000000; +export const MountPassiveDev = /* */ 0b100000000000000000000000; // Groups of flags that are used in the commit phase to skip over trees that // don't contain effects, by checking subtreeFlags. diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 8cbcc58191b83b..7f46304c635173 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -44,6 +44,7 @@ import { SyncLane, NoLanes, isSubsetOfLanes, + includesBlockingLane, mergeLanes, removeLanes, intersectLanes, @@ -68,6 +69,7 @@ import { PassiveStatic as PassiveStaticEffect, StaticMask as StaticMaskEffect, Update as UpdateEffect, + StoreConsistency, } from './ReactFiberFlags'; import { HasEffect as HookHasEffect, @@ -166,7 +168,15 @@ type StoreInstance = {| getSnapshot: () => T, |}; -export type FunctionComponentUpdateQueue = {|lastEffect: Effect | null|}; +type StoreConsistencyCheck = {| + value: T, + getSnapshot: () => T, +|}; + +export type FunctionComponentUpdateQueue = {| + lastEffect: Effect | null, + stores: Array> | null, +|}; type BasicStateAction = (S => S) | S; @@ -689,6 +699,7 @@ function updateWorkInProgressHook(): Hook { function createFunctionComponentUpdateQueue(): FunctionComponentUpdateQueue { return { lastEffect: null, + stores: null, }; } @@ -1289,17 +1300,25 @@ function mountSyncExternalStore( // don't need to set a static flag, either. // TODO: We can move this to the passive phase once we add a pre-commit // consistency check. See the next comment. - fiber.flags |= UpdateEffect; + fiber.flags |= PassiveEffect; pushEffect( - HookHasEffect | HookLayout, + HookHasEffect | HookPassive, updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), undefined, null, ); - // TODO: Unless this is a synchronous render, schedule a consistency check. - // Right before committing, we will walk the tree and check if any of the - // stores were mutated. - // pushConsistencyCheck(inst, getSnapshot, nextSnapshot); + + // Unless we're rendering a blocking lane, schedule a consistency check. Right + // before committing, we will walk the tree and check if any of the stores + // were mutated. + const root: FiberRoot | null = getWorkInProgressRoot(); + invariant( + root !== null, + 'Expected a work-in-progress root. This is a bug in React. Please file an issue.', + ); + if (!includesBlockingLane(root, renderLanes)) { + pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot); + } return nextSnapshot; } @@ -1348,36 +1367,69 @@ function updateSyncExternalStore( (workInProgressHook !== null && workInProgressHook.memoizedState.tag & HookHasEffect) ) { - // TODO: We can move this to the passive phase once we add a pre-commit - // consistency check. See the next comment. - fiber.flags |= UpdateEffect; + fiber.flags |= PassiveEffect; pushEffect( - HookHasEffect | HookLayout, + HookHasEffect | HookPassive, updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), undefined, null, ); - // TODO: Unless this is a synchronous render, schedule a consistency check. + // Unless we're rendering a blocking lane, schedule a consistency check. // Right before committing, we will walk the tree and check if any of the // stores were mutated. - // pushConsistencyCheck(inst, getSnapshot, nextSnapshot); + const root: FiberRoot | null = getWorkInProgressRoot(); + invariant( + root !== null, + 'Expected a work-in-progress root. This is a bug in React. Please file an issue.', + ); + if (!includesBlockingLane(root, renderLanes)) { + pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot); + } } return nextSnapshot; } +function pushStoreConsistencyCheck( + fiber: Fiber, + getSnapshot: () => T, + renderedSnapshot: T, +) { + fiber.flags |= StoreConsistency; + const check: StoreConsistencyCheck = { + getSnapshot, + value: renderedSnapshot, + }; + let componentUpdateQueue: null | FunctionComponentUpdateQueue = (currentlyRenderingFiber.updateQueue: any); + if (componentUpdateQueue === null) { + componentUpdateQueue = createFunctionComponentUpdateQueue(); + currentlyRenderingFiber.updateQueue = (componentUpdateQueue: any); + componentUpdateQueue.stores = [check]; + } else { + const stores = componentUpdateQueue.stores; + if (stores === null) { + componentUpdateQueue.stores = [check]; + } else { + stores.push(check); + } + } +} + function updateStoreInstance( fiber: Fiber, inst: StoreInstance, nextSnapshot: T, getSnapshot: () => T, ) { + // These are updated in the passive phase inst.value = nextSnapshot; inst.getSnapshot = getSnapshot; - // TODO: Move the tearing checks to an earlier, pre-commit phase so that the - // layout effects always observe a consistent tree. + // Something may have been mutated in between render and commit. This could + // have been in an event that fired before the passive effects, or it could + // have been in a layout effect. In that case, we would have used the old + // snapsho and getSnapshot values to bail out. We need to check one more time. if (checkIfSnapshotChanged(inst)) { // Force a re-render. forceStoreRerender(fiber); @@ -1393,11 +1445,6 @@ function subscribeToStore(fiber, inst, subscribe) { forceStoreRerender(fiber); } }; - // Check for changes right before subscribing. Subsequent changes will be - // detected in the subscription handler. - // TODO: Once updateStoreInstance is moved to the passive phase, we can rely - // on that check instead of checking again here. - handleStoreChange(); // Subscribe to the store and return a clean-up function. return subscribe(handleStoreChange); } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 41525872d531d7..cbade34d43d11f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -44,6 +44,7 @@ import { SyncLane, NoLanes, isSubsetOfLanes, + includesBlockingLane, mergeLanes, removeLanes, intersectLanes, @@ -68,6 +69,7 @@ import { PassiveStatic as PassiveStaticEffect, StaticMask as StaticMaskEffect, Update as UpdateEffect, + StoreConsistency, } from './ReactFiberFlags'; import { HasEffect as HookHasEffect, @@ -166,7 +168,15 @@ type StoreInstance = {| getSnapshot: () => T, |}; -export type FunctionComponentUpdateQueue = {|lastEffect: Effect | null|}; +type StoreConsistencyCheck = {| + value: T, + getSnapshot: () => T, +|}; + +export type FunctionComponentUpdateQueue = {| + lastEffect: Effect | null, + stores: Array> | null, +|}; type BasicStateAction = (S => S) | S; @@ -689,6 +699,7 @@ function updateWorkInProgressHook(): Hook { function createFunctionComponentUpdateQueue(): FunctionComponentUpdateQueue { return { lastEffect: null, + stores: null, }; } @@ -1289,17 +1300,25 @@ function mountSyncExternalStore( // don't need to set a static flag, either. // TODO: We can move this to the passive phase once we add a pre-commit // consistency check. See the next comment. - fiber.flags |= UpdateEffect; + fiber.flags |= PassiveEffect; pushEffect( - HookHasEffect | HookLayout, + HookHasEffect | HookPassive, updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), undefined, null, ); - // TODO: Unless this is a synchronous render, schedule a consistency check. - // Right before committing, we will walk the tree and check if any of the - // stores were mutated. - // pushConsistencyCheck(inst, getSnapshot, nextSnapshot); + + // Unless we're rendering a blocking lane, schedule a consistency check. Right + // before committing, we will walk the tree and check if any of the stores + // were mutated. + const root: FiberRoot | null = getWorkInProgressRoot(); + invariant( + root !== null, + 'Expected a work-in-progress root. This is a bug in React. Please file an issue.', + ); + if (!includesBlockingLane(root, renderLanes)) { + pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot); + } return nextSnapshot; } @@ -1348,36 +1367,69 @@ function updateSyncExternalStore( (workInProgressHook !== null && workInProgressHook.memoizedState.tag & HookHasEffect) ) { - // TODO: We can move this to the passive phase once we add a pre-commit - // consistency check. See the next comment. - fiber.flags |= UpdateEffect; + fiber.flags |= PassiveEffect; pushEffect( - HookHasEffect | HookLayout, + HookHasEffect | HookPassive, updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), undefined, null, ); - // TODO: Unless this is a synchronous render, schedule a consistency check. + // Unless we're rendering a blocking lane, schedule a consistency check. // Right before committing, we will walk the tree and check if any of the // stores were mutated. - // pushConsistencyCheck(inst, getSnapshot, nextSnapshot); + const root: FiberRoot | null = getWorkInProgressRoot(); + invariant( + root !== null, + 'Expected a work-in-progress root. This is a bug in React. Please file an issue.', + ); + if (!includesBlockingLane(root, renderLanes)) { + pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot); + } } return nextSnapshot; } +function pushStoreConsistencyCheck( + fiber: Fiber, + getSnapshot: () => T, + renderedSnapshot: T, +) { + fiber.flags |= StoreConsistency; + const check: StoreConsistencyCheck = { + getSnapshot, + value: renderedSnapshot, + }; + let componentUpdateQueue: null | FunctionComponentUpdateQueue = (currentlyRenderingFiber.updateQueue: any); + if (componentUpdateQueue === null) { + componentUpdateQueue = createFunctionComponentUpdateQueue(); + currentlyRenderingFiber.updateQueue = (componentUpdateQueue: any); + componentUpdateQueue.stores = [check]; + } else { + const stores = componentUpdateQueue.stores; + if (stores === null) { + componentUpdateQueue.stores = [check]; + } else { + stores.push(check); + } + } +} + function updateStoreInstance( fiber: Fiber, inst: StoreInstance, nextSnapshot: T, getSnapshot: () => T, ) { + // These are updated in the passive phase inst.value = nextSnapshot; inst.getSnapshot = getSnapshot; - // TODO: Move the tearing checks to an earlier, pre-commit phase so that the - // layout effects always observe a consistent tree. + // Something may have been mutated in between render and commit. This could + // have been in an event that fired before the passive effects, or it could + // have been in a layout effect. In that case, we would have used the old + // snapsho and getSnapshot values to bail out. We need to check one more time. if (checkIfSnapshotChanged(inst)) { // Force a re-render. forceStoreRerender(fiber); @@ -1393,11 +1445,6 @@ function subscribeToStore(fiber, inst, subscribe) { forceStoreRerender(fiber); } }; - // Check for changes right before subscribing. Subsequent changes will be - // detected in the subscription handler. - // TODO: Once updateStoreInstance is moved to the passive phase, we can rely - // on that check instead of checking again here. - handleStoreChange(); // Subscribe to the store and return a clean-up function. return subscribe(handleStoreChange); } diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 2ad6b6a69a4952..ad124a432a4a2b 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -453,27 +453,26 @@ export function includesOnlyTransitions(lanes: Lanes) { return (lanes & TransitionLanes) === lanes; } -export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) { - if ((lanes & root.expiredLanes) !== NoLanes) { - // At least one of these lanes expired. To prevent additional starvation, - // finish rendering without yielding execution. - return false; - } - +export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { if ( allowConcurrentByDefault && (root.current.mode & ConcurrentUpdatesByDefaultMode) !== NoMode ) { // Concurrent updates by default always use time slicing. - return true; + return false; } - const SyncDefaultLanes = InputContinuousHydrationLane | InputContinuousLane | DefaultHydrationLane | DefaultLane; - return (lanes & SyncDefaultLanes) === NoLanes; + return (lanes & SyncDefaultLanes) !== NoLanes; +} + +export function includesExpiredLane(root: FiberRoot, lanes: Lanes) { + // This is a separate check from includesBlockingLane because a lane can + // expire after a render has already started. + return (lanes & root.expiredLanes) !== NoLanes; } export function isTransitionLane(lane: Lane) { diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 3e704f54e6761e..4a064a38465155 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -453,27 +453,26 @@ export function includesOnlyTransitions(lanes: Lanes) { return (lanes & TransitionLanes) === lanes; } -export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) { - if ((lanes & root.expiredLanes) !== NoLanes) { - // At least one of these lanes expired. To prevent additional starvation, - // finish rendering without yielding execution. - return false; - } - +export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { if ( allowConcurrentByDefault && (root.current.mode & ConcurrentUpdatesByDefaultMode) !== NoMode ) { // Concurrent updates by default always use time slicing. - return true; + return false; } - const SyncDefaultLanes = InputContinuousHydrationLane | InputContinuousLane | DefaultHydrationLane | DefaultLane; - return (lanes & SyncDefaultLanes) === NoLanes; + return (lanes & SyncDefaultLanes) !== NoLanes; +} + +export function includesExpiredLane(root: FiberRoot, lanes: Lanes) { + // This is a separate check from includesBlockingLane because a lane can + // expire after a render has already started. + return (lanes & root.expiredLanes) !== NoLanes; } export function isTransitionLane(lane: Lane) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index dc79e39e9d6a1b..00d38a2cbe8d8d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -13,6 +13,7 @@ import type {Lanes, Lane} from './ReactFiberLane.new'; import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; import type {StackCursor} from './ReactFiberStack.new'; import type {Flags} from './ReactFiberFlags'; +import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; import { warnAboutDeprecatedLifecycles, @@ -34,6 +35,7 @@ import { } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; +import is from 'shared/objectIs'; import { // Aliased because `act` will override and push to an internal queue @@ -116,6 +118,7 @@ import { NoFlags, Placement, Incomplete, + StoreConsistency, HostEffectMask, Hydrating, BeforeMutationMask, @@ -140,7 +143,8 @@ import { includesNonIdleWork, includesOnlyRetries, includesOnlyTransitions, - shouldTimeSlice, + includesBlockingLane, + includesExpiredLane, getNextLanes, markStarvedLanesAsExpired, getLanesToRetrySynchronouslyOnError, @@ -769,39 +773,25 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // TODO: We only check `didTimeout` defensively, to account for a Scheduler // bug we're still investigating. Once the bug in Scheduler is fixed, // we can remove this, since we track expiration ourselves. - let exitStatus = - shouldTimeSlice(root, lanes) && - (disableSchedulerTimeoutInWorkLoop || !didTimeout) - ? renderRootConcurrent(root, lanes) - : renderRootSync(root, lanes); + const shouldTimeSlice = + !includesBlockingLane(root, lanes) && + !includesExpiredLane(root, lanes) && + (disableSchedulerTimeoutInWorkLoop || !didTimeout); + let exitStatus = shouldTimeSlice + ? renderRootConcurrent(root, lanes) + : renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { - const prevExecutionContext = executionContext; - executionContext |= RetryAfterError; - - // If an error occurred during hydration, - // discard server response and fall back to client side render. - if (root.hydrate) { - root.hydrate = false; - if (__DEV__) { - errorHydratingContainer(root.containerInfo); - } - clearContainer(root.containerInfo); - } - - // If something threw an error, try rendering one more time. We'll render - // synchronously to block concurrent data mutations, and we'll includes - // all pending updates are included. If it still fails after the second - // attempt, we'll give up and commit the resulting tree. + // If something threw an error, try rendering one more time. We'll + // render synchronously to block concurrent data mutations, and we'll + // includes all pending updates are included. If it still fails after + // the second attempt, we'll give up and commit the resulting tree. const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); if (errorRetryLanes !== NoLanes) { lanes = errorRetryLanes; - exitStatus = renderRootSync(root, errorRetryLanes); + exitStatus = recoverFromConcurrentError(root, errorRetryLanes); } - - executionContext = prevExecutionContext; } - if (exitStatus === RootFatalErrored) { const fatalError = workInProgressRootFatalError; prepareFreshStack(root, NoLanes); @@ -810,9 +800,42 @@ function performConcurrentWorkOnRoot(root, didTimeout) { throw fatalError; } + // Check if this render may have yielded to a concurrent event, and if so, + // confirm that any newly rendered stores are consistent. + // TODO: It's possible that even a concurrent render may never have yielded + // to the main thread, if it was fast enough, or if it expired. We could + // skip the consistency check in that case, too. + const renderWasConcurrent = !includesBlockingLane(root, lanes); + const finishedWork: Fiber = (root.current.alternate: any); + if ( + renderWasConcurrent && + !isRenderConsistentWithExternalStores(finishedWork) + ) { + // A store was mutated in an interleaved event. Render again, + // synchronously, to block further mutations. + exitStatus = renderRootSync(root, lanes); + + // We need to check again if something threw + if (exitStatus === RootErrored) { + const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); + if (errorRetryLanes !== NoLanes) { + lanes = errorRetryLanes; + exitStatus = recoverFromConcurrentError(root, errorRetryLanes); + // We assume the tree is now consistent because we didn't yield to any + // concurrent events. + } + } + if (exitStatus === RootFatalErrored) { + const fatalError = workInProgressRootFatalError; + prepareFreshStack(root, NoLanes); + markRootSuspended(root, lanes); + ensureRootIsScheduled(root, now()); + throw fatalError; + } + } + // We now have a consistent tree. The next step is either to commit it, // or, if something suspended, wait to commit it after a timeout. - const finishedWork: Fiber = (root.current.alternate: any); root.finishedWork = finishedWork; root.finishedLanes = lanes; finishConcurrentRender(root, exitStatus, lanes); @@ -827,6 +850,27 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } +function recoverFromConcurrentError(root, errorRetryLanes) { + const prevExecutionContext = executionContext; + executionContext |= RetryAfterError; + + // If an error occurred during hydration, discard server response and fall + // back to client side render. + if (root.hydrate) { + root.hydrate = false; + if (__DEV__) { + errorHydratingContainer(root.containerInfo); + } + clearContainer(root.containerInfo); + } + + const exitStatus = renderRootSync(root, errorRetryLanes); + + executionContext = prevExecutionContext; + + return exitStatus; +} + function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: @@ -939,6 +983,58 @@ function finishConcurrentRender(root, exitStatus, lanes) { } } +function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean { + // Search the rendered tree for external store reads, and check whether the + // stores were mutated in a concurrent event. Intentionally using a iterative + // loop instead of recursion so we can exit early. + let node: Fiber = finishedWork; + while (true) { + if (node.flags & StoreConsistency) { + const updateQueue: FunctionComponentUpdateQueue | null = (node.updateQueue: any); + if (updateQueue !== null) { + const checks = updateQueue.stores; + if (checks !== null) { + for (let i = 0; i < checks.length; i++) { + const check = checks[i]; + const getSnapshot = check.getSnapshot; + const renderedValue = check.value; + try { + if (!is(getSnapshot(), renderedValue)) { + // Found an inconsistent store. + return false; + } + } catch (error) { + // If `getSnapshot` throws, return `false`. This will schedule + // a re-render, and the error will be rethrown during render. + return false; + } + } + } + } + } + const child = node.child; + if (node.subtreeFlags & StoreConsistency && child !== null) { + child.return = node; + node = child; + continue; + } + if (node === finishedWork) { + return true; + } + while (node.sibling === null) { + if (node.return === null || node.return === finishedWork) { + return true; + } + node = node.return; + } + node.sibling.return = node.return; + node = node.sibling; + } + // Flow doesn't know this is unreachable, but eslint does + // eslint-disable-next-line no-unreachable + return true; +} + function markRootSuspended(root, suspendedLanes) { // When suspending, we should always exclude lanes that were pinged or (more // rarely, since we try to avoid it) updated during the render phase. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index e0cb491e8214da..2561d865b0e809 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -13,6 +13,7 @@ import type {Lanes, Lane} from './ReactFiberLane.old'; import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; import type {StackCursor} from './ReactFiberStack.old'; import type {Flags} from './ReactFiberFlags'; +import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; import { warnAboutDeprecatedLifecycles, @@ -34,6 +35,7 @@ import { } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; +import is from 'shared/objectIs'; import { // Aliased because `act` will override and push to an internal queue @@ -116,6 +118,7 @@ import { NoFlags, Placement, Incomplete, + StoreConsistency, HostEffectMask, Hydrating, BeforeMutationMask, @@ -140,7 +143,8 @@ import { includesNonIdleWork, includesOnlyRetries, includesOnlyTransitions, - shouldTimeSlice, + includesBlockingLane, + includesExpiredLane, getNextLanes, markStarvedLanesAsExpired, getLanesToRetrySynchronouslyOnError, @@ -769,39 +773,25 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // TODO: We only check `didTimeout` defensively, to account for a Scheduler // bug we're still investigating. Once the bug in Scheduler is fixed, // we can remove this, since we track expiration ourselves. - let exitStatus = - shouldTimeSlice(root, lanes) && - (disableSchedulerTimeoutInWorkLoop || !didTimeout) - ? renderRootConcurrent(root, lanes) - : renderRootSync(root, lanes); + const shouldTimeSlice = + !includesBlockingLane(root, lanes) && + !includesExpiredLane(root, lanes) && + (disableSchedulerTimeoutInWorkLoop || !didTimeout); + let exitStatus = shouldTimeSlice + ? renderRootConcurrent(root, lanes) + : renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { - const prevExecutionContext = executionContext; - executionContext |= RetryAfterError; - - // If an error occurred during hydration, - // discard server response and fall back to client side render. - if (root.hydrate) { - root.hydrate = false; - if (__DEV__) { - errorHydratingContainer(root.containerInfo); - } - clearContainer(root.containerInfo); - } - - // If something threw an error, try rendering one more time. We'll render - // synchronously to block concurrent data mutations, and we'll includes - // all pending updates are included. If it still fails after the second - // attempt, we'll give up and commit the resulting tree. + // If something threw an error, try rendering one more time. We'll + // render synchronously to block concurrent data mutations, and we'll + // includes all pending updates are included. If it still fails after + // the second attempt, we'll give up and commit the resulting tree. const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); if (errorRetryLanes !== NoLanes) { lanes = errorRetryLanes; - exitStatus = renderRootSync(root, errorRetryLanes); + exitStatus = recoverFromConcurrentError(root, errorRetryLanes); } - - executionContext = prevExecutionContext; } - if (exitStatus === RootFatalErrored) { const fatalError = workInProgressRootFatalError; prepareFreshStack(root, NoLanes); @@ -810,9 +800,42 @@ function performConcurrentWorkOnRoot(root, didTimeout) { throw fatalError; } + // Check if this render may have yielded to a concurrent event, and if so, + // confirm that any newly rendered stores are consistent. + // TODO: It's possible that even a concurrent render may never have yielded + // to the main thread, if it was fast enough, or if it expired. We could + // skip the consistency check in that case, too. + const renderWasConcurrent = !includesBlockingLane(root, lanes); + const finishedWork: Fiber = (root.current.alternate: any); + if ( + renderWasConcurrent && + !isRenderConsistentWithExternalStores(finishedWork) + ) { + // A store was mutated in an interleaved event. Render again, + // synchronously, to block further mutations. + exitStatus = renderRootSync(root, lanes); + + // We need to check again if something threw + if (exitStatus === RootErrored) { + const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); + if (errorRetryLanes !== NoLanes) { + lanes = errorRetryLanes; + exitStatus = recoverFromConcurrentError(root, errorRetryLanes); + // We assume the tree is now consistent because we didn't yield to any + // concurrent events. + } + } + if (exitStatus === RootFatalErrored) { + const fatalError = workInProgressRootFatalError; + prepareFreshStack(root, NoLanes); + markRootSuspended(root, lanes); + ensureRootIsScheduled(root, now()); + throw fatalError; + } + } + // We now have a consistent tree. The next step is either to commit it, // or, if something suspended, wait to commit it after a timeout. - const finishedWork: Fiber = (root.current.alternate: any); root.finishedWork = finishedWork; root.finishedLanes = lanes; finishConcurrentRender(root, exitStatus, lanes); @@ -827,6 +850,27 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } +function recoverFromConcurrentError(root, errorRetryLanes) { + const prevExecutionContext = executionContext; + executionContext |= RetryAfterError; + + // If an error occurred during hydration, discard server response and fall + // back to client side render. + if (root.hydrate) { + root.hydrate = false; + if (__DEV__) { + errorHydratingContainer(root.containerInfo); + } + clearContainer(root.containerInfo); + } + + const exitStatus = renderRootSync(root, errorRetryLanes); + + executionContext = prevExecutionContext; + + return exitStatus; +} + function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: @@ -939,6 +983,58 @@ function finishConcurrentRender(root, exitStatus, lanes) { } } +function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean { + // Search the rendered tree for external store reads, and check whether the + // stores were mutated in a concurrent event. Intentionally using a iterative + // loop instead of recursion so we can exit early. + let node: Fiber = finishedWork; + while (true) { + if (node.flags & StoreConsistency) { + const updateQueue: FunctionComponentUpdateQueue | null = (node.updateQueue: any); + if (updateQueue !== null) { + const checks = updateQueue.stores; + if (checks !== null) { + for (let i = 0; i < checks.length; i++) { + const check = checks[i]; + const getSnapshot = check.getSnapshot; + const renderedValue = check.value; + try { + if (!is(getSnapshot(), renderedValue)) { + // Found an inconsistent store. + return false; + } + } catch (error) { + // If `getSnapshot` throws, return `false`. This will schedule + // a re-render, and the error will be rethrown during render. + return false; + } + } + } + } + } + const child = node.child; + if (node.subtreeFlags & StoreConsistency && child !== null) { + child.return = node; + node = child; + continue; + } + if (node === finishedWork) { + return true; + } + while (node.sibling === null) { + if (node.return === null || node.return === finishedWork) { + return true; + } + node = node.return; + } + node.sibling.return = node.return; + node = node.sibling; + } + // Flow doesn't know this is unreachable, but eslint does + // eslint-disable-next-line no-unreachable + return true; +} + function markRootSuspended(root, suspendedLanes) { // When suspending, we should always exclude lanes that were pinged or (more // rarely, since we try to avoid it) updated during the render phase. diff --git a/packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js b/packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js new file mode 100644 index 00000000000000..b402c69e02ad96 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js @@ -0,0 +1,171 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let useSyncExternalStore; +let React; +let ReactNoop; +let Scheduler; +let act; +let useLayoutEffect; +let forwardRef; +let useImperativeHandle; +let useRef; + +// This tests the native useSyncExternalStore implementation, not the shim. +// Tests that apply to both the native implementation and the shim should go +// into useSyncExternalStoreShared-test.js. The reason they are separate is +// because at some point we may start running the shared tests against vendored +// React DOM versions (16, 17, etc) instead of React Noop. +describe('useSyncExternalStore', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + useLayoutEffect = React.useLayoutEffect; + useImperativeHandle = React.useImperativeHandle; + forwardRef = React.forwardRef; + useRef = React.useRef; + useSyncExternalStore = React.unstable_useSyncExternalStore; + + act = require('jest-react').act; + }); + + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return text; + } + + function createExternalStore(initialState) { + const listeners = new Set(); + let currentState = initialState; + return { + set(text) { + currentState = text; + ReactNoop.batchedUpdates(() => { + listeners.forEach(listener => listener()); + }); + }, + subscribe(listener) { + listeners.add(listener); + return () => listeners.delete(listener); + }, + getState() { + return currentState; + }, + getSubscriberCount() { + return listeners.size; + }, + }; + } + + // @gate supportsNativeUseSyncExternalStore + test( + 'detects interleaved mutations during a concurrent read before ' + + 'layout effects fire', + async () => { + const store1 = createExternalStore(0); + const store2 = createExternalStore(0); + + const Child = forwardRef(({store, label}, ref) => { + const value = useSyncExternalStore(store.subscribe, store.getState); + useImperativeHandle( + ref, + () => { + return value; + }, + [], + ); + return ; + }); + + function App({store}) { + const refA = useRef(null); + const refB = useRef(null); + const refC = useRef(null); + useLayoutEffect(() => { + // This layout effect reads children that depend on an external store. + // This demostrates whether the children are consistent when the + // layout phase runs. + const aText = refA.current; + const bText = refB.current; + const cText = refC.current; + Scheduler.unstable_yieldValue( + `Children observed during layout: A${aText}B${bText}C${cText}`, + ); + }); + return ( + <> + + + + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + + // Start a concurrent render that reads from the store, then yield. + expect(Scheduler).toFlushAndYieldThrough(['A0', 'B0']); + + // During an interleaved event, the store is mutated. + store1.set(1); + + // Then we continue rendering. + expect(Scheduler).toFlushAndYield([ + // C reads a newer value from the store than A or B, which means they + // are inconsistent. + 'C1', + + // Before committing the layout effects, React detects that the store + // has been mutated. So it throws out the entire completed tree and + // re-renders the new values. + 'A1', + 'B1', + 'C1', + // The layout effects reads consistent children. + 'Children observed during layout: A1B1C1', + ]); + }); + + // Now we're going test the same thing during an update that + // switches stores. + await act(async () => { + root.render(); + + // Start a concurrent render that reads from the store, then yield. + expect(Scheduler).toFlushAndYieldThrough(['A0', 'B0']); + + // During an interleaved event, the store is mutated. + store2.set(1); + + // Then we continue rendering. + expect(Scheduler).toFlushAndYield([ + // C reads a newer value from the store than A or B, which means they + // are inconsistent. + 'C1', + + // Before committing the layout effects, React detects that the store + // has been mutated. So it throws out the entire completed tree and + // re-renders the new values. + 'A1', + 'B1', + 'C1', + // The layout effects reads consistent children. + 'Children observed during layout: A1B1C1', + ]); + }); + }, + ); +}); diff --git a/scripts/jest/TestFlags.js b/scripts/jest/TestFlags.js index 9dbea6326ddecd..c1bd03e00b1a9a 100644 --- a/scripts/jest/TestFlags.js +++ b/scripts/jest/TestFlags.js @@ -84,6 +84,10 @@ function getTestFlags() { source: !process.env.IS_BUILD, www, + // This isn't a flag, just a useful alias for tests. Remove once + // useSyncExternalStore lands in the `next` channel. + supportsNativeUseSyncExternalStore: __EXPERIMENTAL__ || www, + // If there's a naming conflict between scheduler and React feature flags, the // React ones take precedence. // TODO: Maybe we should error on conflicts? Or we could namespace