Skip to content

Commit c71d3ab

Browse files
acdliteacdlite
committed
Handle render phase updates explicitly
We fire a warning in development if a component is updated during the render phase (with the exception of local hook updates, which have their own defined behavior). Because it's not a supported React pattern, we don't have that many tests that trigger this path. But it is meant to have reasonable semantics when it does happen, so that if it accidentally ships to production, the app doesn't crash unnecessarily. The behavior is not super well-defined, though. There are also some _internal_ React implementation details that intentionally to rely on this behavior. Most prominently, selective hydration and useOpaqueIdentifier. I need to tweak the behavior of render phase updates slightly as part of a fix for useOpaqueIdentifier. This shouldn't cause a user-facing change in behavior outside of useOpaqueIdentifier, but it does require that we explicitly model render phase updates.
1 parent 8ee4ff8 commit c71d3ab

File tree

2 files changed

+148
-124
lines changed

2 files changed

+148
-124
lines changed

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js‎

Lines changed: 74 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -454,86 +454,99 @@ export function scheduleUpdateOnFiber(
454454
eventTime: number,
455455
): FiberRoot | null {
456456
checkForNestedUpdates();
457-
warnAboutRenderPhaseUpdatesInDEV(fiber);
458457

459458
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
460459
if (root === null) {
461460
return null;
462461
}
463462

464-
if (enableUpdaterTracking) {
465-
if (isDevToolsPresent) {
466-
addFiberToLanesMap(root, fiber, lane);
467-
}
468-
}
469-
470463
// Mark that the root has a pending update.
471464
markRootUpdated(root, lane, eventTime);
472465

473-
if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) {
474-
if (
475-
(executionContext & CommitContext) !== NoContext &&
476-
root === rootCommittingMutationOrLayoutEffects
477-
) {
478-
if (fiber.mode & ProfileMode) {
479-
let current = fiber;
480-
while (current !== null) {
481-
if (current.tag === Profiler) {
482-
const {id, onNestedUpdateScheduled} = current.memoizedProps;
483-
if (typeof onNestedUpdateScheduled === 'function') {
484-
onNestedUpdateScheduled(id);
466+
if (
467+
(executionContext & RenderContext) !== NoLanes &&
468+
root === workInProgressRoot
469+
) {
470+
// This update was dispatched during the render phase. This is a mistake
471+
// if the update originates from user space (with the exception of local
472+
// hook updates, which are handled differently and don't reach this
473+
// function), but there are some internal React features that use this as
474+
// an implementation detail, like selective hydration
475+
// and useOpaqueIdentifier.
476+
warnAboutRenderPhaseUpdatesInDEV(fiber);
477+
} else {
478+
// This is a normal update, scheduled from outside the render phase. For
479+
// example, during an input event.
480+
if (enableUpdaterTracking) {
481+
if (isDevToolsPresent) {
482+
addFiberToLanesMap(root, fiber, lane);
483+
}
484+
}
485+
486+
if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) {
487+
if (
488+
(executionContext & CommitContext) !== NoContext &&
489+
root === rootCommittingMutationOrLayoutEffects
490+
) {
491+
if (fiber.mode & ProfileMode) {
492+
let current = fiber;
493+
while (current !== null) {
494+
if (current.tag === Profiler) {
495+
const {id, onNestedUpdateScheduled} = current.memoizedProps;
496+
if (typeof onNestedUpdateScheduled === 'function') {
497+
onNestedUpdateScheduled(id);
498+
}
485499
}
500+
current = current.return;
486501
}
487-
current = current.return;
488502
}
489503
}
490504
}
491-
}
492505

493-
// TODO: Consolidate with `isInterleavedUpdate` check
494-
if (root === workInProgressRoot) {
495-
// Received an update to a tree that's in the middle of rendering. Mark
496-
// that there was an interleaved update work on this root. Unless the
497-
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
498-
// phase update. In that case, we don't treat render phase updates as if
499-
// they were interleaved, for backwards compat reasons.
506+
// TODO: Consolidate with `isInterleavedUpdate` check
507+
if (root === workInProgressRoot) {
508+
// Received an update to a tree that's in the middle of rendering. Mark
509+
// that there was an interleaved update work on this root. Unless the
510+
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
511+
// phase update. In that case, we don't treat render phase updates as if
512+
// they were interleaved, for backwards compat reasons.
513+
if (
514+
deferRenderPhaseUpdateToNextBatch ||
515+
(executionContext & RenderContext) === NoContext
516+
) {
517+
workInProgressRootUpdatedLanes = mergeLanes(
518+
workInProgressRootUpdatedLanes,
519+
lane,
520+
);
521+
}
522+
if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
523+
// The root already suspended with a delay, which means this render
524+
// definitely won't finish. Since we have a new update, let's mark it as
525+
// suspended now, right before marking the incoming update. This has the
526+
// effect of interrupting the current render and switching to the update.
527+
// TODO: Make sure this doesn't override pings that happen while we've
528+
// already started rendering.
529+
markRootSuspended(root, workInProgressRootRenderLanes);
530+
}
531+
}
532+
533+
ensureRootIsScheduled(root, eventTime);
500534
if (
501-
deferRenderPhaseUpdateToNextBatch ||
502-
(executionContext & RenderContext) === NoContext
535+
lane === SyncLane &&
536+
executionContext === NoContext &&
537+
(fiber.mode & ConcurrentMode) === NoMode &&
538+
// Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
539+
!(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
503540
) {
504-
workInProgressRootUpdatedLanes = mergeLanes(
505-
workInProgressRootUpdatedLanes,
506-
lane,
507-
);
508-
}
509-
if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
510-
// The root already suspended with a delay, which means this render
511-
// definitely won't finish. Since we have a new update, let's mark it as
512-
// suspended now, right before marking the incoming update. This has the
513-
// effect of interrupting the current render and switching to the update.
514-
// TODO: Make sure this doesn't override pings that happen while we've
515-
// already started rendering.
516-
markRootSuspended(root, workInProgressRootRenderLanes);
541+
// Flush the synchronous work now, unless we're already working or inside
542+
// a batch. This is intentionally inside scheduleUpdateOnFiber instead of
543+
// scheduleCallbackForFiber to preserve the ability to schedule a callback
544+
// without immediately flushing it. We only do this for user-initiated
545+
// updates, to preserve historical behavior of legacy mode.
546+
resetRenderTimer();
547+
flushSyncCallbacksOnlyInLegacyMode();
517548
}
518549
}
519-
520-
ensureRootIsScheduled(root, eventTime);
521-
if (
522-
lane === SyncLane &&
523-
executionContext === NoContext &&
524-
(fiber.mode & ConcurrentMode) === NoMode &&
525-
// Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
526-
!(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
527-
) {
528-
// Flush the synchronous work now, unless we're already working or inside
529-
// a batch. This is intentionally inside scheduleUpdateOnFiber instead of
530-
// scheduleCallbackForFiber to preserve the ability to schedule a callback
531-
// without immediately flushing it. We only do this for user-initiated
532-
// updates, to preserve historical behavior of legacy mode.
533-
resetRenderTimer();
534-
flushSyncCallbacksOnlyInLegacyMode();
535-
}
536-
537550
return root;
538551
}
539552

@@ -2697,7 +2710,6 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
26972710
if (__DEV__) {
26982711
if (
26992712
ReactCurrentDebugFiberIsRenderingInDEV &&
2700-
(executionContext & RenderContext) !== NoContext &&
27012713
!getIsUpdatingOpaqueValueInRenderPhaseInDEV()
27022714
) {
27032715
switch (fiber.tag) {

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js‎

Lines changed: 74 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -454,86 +454,99 @@ export function scheduleUpdateOnFiber(
454454
eventTime: number,
455455
): FiberRoot | null {
456456
checkForNestedUpdates();
457-
warnAboutRenderPhaseUpdatesInDEV(fiber);
458457

459458
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
460459
if (root === null) {
461460
return null;
462461
}
463462

464-
if (enableUpdaterTracking) {
465-
if (isDevToolsPresent) {
466-
addFiberToLanesMap(root, fiber, lane);
467-
}
468-
}
469-
470463
// Mark that the root has a pending update.
471464
markRootUpdated(root, lane, eventTime);
472465

473-
if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) {
474-
if (
475-
(executionContext & CommitContext) !== NoContext &&
476-
root === rootCommittingMutationOrLayoutEffects
477-
) {
478-
if (fiber.mode & ProfileMode) {
479-
let current = fiber;
480-
while (current !== null) {
481-
if (current.tag === Profiler) {
482-
const {id, onNestedUpdateScheduled} = current.memoizedProps;
483-
if (typeof onNestedUpdateScheduled === 'function') {
484-
onNestedUpdateScheduled(id);
466+
if (
467+
(executionContext & RenderContext) !== NoLanes &&
468+
root === workInProgressRoot
469+
) {
470+
// This update was dispatched during the render phase. This is a mistake
471+
// if the update originates from user space (with the exception of local
472+
// hook updates, which are handled differently and don't reach this
473+
// function), but there are some internal React features that use this as
474+
// an implementation detail, like selective hydration
475+
// and useOpaqueIdentifier.
476+
warnAboutRenderPhaseUpdatesInDEV(fiber);
477+
} else {
478+
// This is a normal update, scheduled from outside the render phase. For
479+
// example, during an input event.
480+
if (enableUpdaterTracking) {
481+
if (isDevToolsPresent) {
482+
addFiberToLanesMap(root, fiber, lane);
483+
}
484+
}
485+
486+
if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) {
487+
if (
488+
(executionContext & CommitContext) !== NoContext &&
489+
root === rootCommittingMutationOrLayoutEffects
490+
) {
491+
if (fiber.mode & ProfileMode) {
492+
let current = fiber;
493+
while (current !== null) {
494+
if (current.tag === Profiler) {
495+
const {id, onNestedUpdateScheduled} = current.memoizedProps;
496+
if (typeof onNestedUpdateScheduled === 'function') {
497+
onNestedUpdateScheduled(id);
498+
}
485499
}
500+
current = current.return;
486501
}
487-
current = current.return;
488502
}
489503
}
490504
}
491-
}
492505

493-
// TODO: Consolidate with `isInterleavedUpdate` check
494-
if (root === workInProgressRoot) {
495-
// Received an update to a tree that's in the middle of rendering. Mark
496-
// that there was an interleaved update work on this root. Unless the
497-
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
498-
// phase update. In that case, we don't treat render phase updates as if
499-
// they were interleaved, for backwards compat reasons.
506+
// TODO: Consolidate with `isInterleavedUpdate` check
507+
if (root === workInProgressRoot) {
508+
// Received an update to a tree that's in the middle of rendering. Mark
509+
// that there was an interleaved update work on this root. Unless the
510+
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
511+
// phase update. In that case, we don't treat render phase updates as if
512+
// they were interleaved, for backwards compat reasons.
513+
if (
514+
deferRenderPhaseUpdateToNextBatch ||
515+
(executionContext & RenderContext) === NoContext
516+
) {
517+
workInProgressRootUpdatedLanes = mergeLanes(
518+
workInProgressRootUpdatedLanes,
519+
lane,
520+
);
521+
}
522+
if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
523+
// The root already suspended with a delay, which means this render
524+
// definitely won't finish. Since we have a new update, let's mark it as
525+
// suspended now, right before marking the incoming update. This has the
526+
// effect of interrupting the current render and switching to the update.
527+
// TODO: Make sure this doesn't override pings that happen while we've
528+
// already started rendering.
529+
markRootSuspended(root, workInProgressRootRenderLanes);
530+
}
531+
}
532+
533+
ensureRootIsScheduled(root, eventTime);
500534
if (
501-
deferRenderPhaseUpdateToNextBatch ||
502-
(executionContext & RenderContext) === NoContext
535+
lane === SyncLane &&
536+
executionContext === NoContext &&
537+
(fiber.mode & ConcurrentMode) === NoMode &&
538+
// Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
539+
!(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
503540
) {
504-
workInProgressRootUpdatedLanes = mergeLanes(
505-
workInProgressRootUpdatedLanes,
506-
lane,
507-
);
508-
}
509-
if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
510-
// The root already suspended with a delay, which means this render
511-
// definitely won't finish. Since we have a new update, let's mark it as
512-
// suspended now, right before marking the incoming update. This has the
513-
// effect of interrupting the current render and switching to the update.
514-
// TODO: Make sure this doesn't override pings that happen while we've
515-
// already started rendering.
516-
markRootSuspended(root, workInProgressRootRenderLanes);
541+
// Flush the synchronous work now, unless we're already working or inside
542+
// a batch. This is intentionally inside scheduleUpdateOnFiber instead of
543+
// scheduleCallbackForFiber to preserve the ability to schedule a callback
544+
// without immediately flushing it. We only do this for user-initiated
545+
// updates, to preserve historical behavior of legacy mode.
546+
resetRenderTimer();
547+
flushSyncCallbacksOnlyInLegacyMode();
517548
}
518549
}
519-
520-
ensureRootIsScheduled(root, eventTime);
521-
if (
522-
lane === SyncLane &&
523-
executionContext === NoContext &&
524-
(fiber.mode & ConcurrentMode) === NoMode &&
525-
// Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
526-
!(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
527-
) {
528-
// Flush the synchronous work now, unless we're already working or inside
529-
// a batch. This is intentionally inside scheduleUpdateOnFiber instead of
530-
// scheduleCallbackForFiber to preserve the ability to schedule a callback
531-
// without immediately flushing it. We only do this for user-initiated
532-
// updates, to preserve historical behavior of legacy mode.
533-
resetRenderTimer();
534-
flushSyncCallbacksOnlyInLegacyMode();
535-
}
536-
537550
return root;
538551
}
539552

@@ -2697,7 +2710,6 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
26972710
if (__DEV__) {
26982711
if (
26992712
ReactCurrentDebugFiberIsRenderingInDEV &&
2700-
(executionContext & RenderContext) !== NoContext &&
27012713
!getIsUpdatingOpaqueValueInRenderPhaseInDEV()
27022714
) {
27032715
switch (fiber.tag) {

0 commit comments

Comments
 (0)