Bugfix: Nested useOpaqueIdentifier references (#22553)
* 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. * Bugfix: Nested useOpaqueIdentifier calls Fixes an issue where multiple useOpaqueIdentifier hooks are upgraded to client ids within the same render. The way the upgrade works is that useOpaqueIdentifier schedules a render phase update then throws an error to trigger React's error recovery mechanism. The normal error recovery mechanism is designed for errors that occur as a result of interleaved mutations, so we usually only retry a single time, synchronously, before giving up. useOpaqueIdentifier is different because the error its throws when upgrading is not caused by an interleaved mutation. Rather, it happens when an ID is referenced for the first time inside a client-rendered tree (i.e. sommething that wasn't part of the initial server render). The fact that it relies on the error recovery mechanism is an implementation detail. And a single recovery attempt may be insufficient. For example, if a parent and a child component may reference different ids, and both are mounted as a result of the same client update, that will trigger two separate error recovery attempts. Because render phase updates are not allowed when triggered from userspace — we log a warning in developement to prevent them — we can assume that if something does update during the render phase, it is one of our "legit" implementation details like useOpaqueIdentifier. So we can keep retrying until we succeed — up to a limit, to protect against inifite loops. I chose 50 since that's the limit we use for commit phase updates.
Andrew Clark committed
Oct 13, 2021 at 18:44 UTC
7142d110b00ecb5bb7566128c431fee02963e543
4 files changed
+314
-166
packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js
+35
@@ -1975,5 +1975,40 @@ describe('ReactDOMServerHooks', () => {
1975
container.getElementsByTagName('span')[0].getAttribute('id'),
1976
).not.toBeNull();
1977
});
1978
+
1979
+ it('useOpaqueIdentifier with multiple ids in nested components', async () => {
1980
+ function DivWithId({id, children}) {
1981
+ return <div id={id}>{children}</div>;
1982
+ }
1983
+
1984
+ let setShowMore;
1985
+ function App() {
1986
+ const outerId = useOpaqueIdentifier();
1987
+ const innerId = useOpaqueIdentifier();
1988
+ const [showMore, _setShowMore] = useState(false);
1989
+ setShowMore = _setShowMore;
1990
+ return showMore ? (
1991
+ <DivWithId id={outerId}>
1992
+ <DivWithId id={innerId} />
1993
+ </DivWithId>
1994
+ ) : null;
1995
+ }
1996
+
1997
+ const container = document.createElement('div');
1998
+ container.innerHTML = ReactDOMServer.renderToString(<App />);
1999
+
2000
+ await act(async () => {
2001
+ ReactDOM.hydrateRoot(container, <App />);
2002
+ });
2003
+
2004
+ // Show additional content that wasn't part of the initial server-
2005
+ // rendered repsonse.
2006
+ await act(async () => {
2007
+ setShowMore(true);
2008
+ });
2009
+ const [div1, div2] = container.getElementsByTagName('div');
2010
+ expect(typeof div1.getAttribute('id')).toBe('string');
2011
+ expect(typeof div2.getAttribute('id')).toBe('string');
2012
+ });
2013
});
2014
});
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+110
-83
@@ -294,7 +294,9 @@ let workInProgressRootIncludedLanes: Lanes = NoLanes;
294
// includes unprocessed updates, not work in bailed out children.
295
let workInProgressRootSkippedLanes: Lanes = NoLanes;
296
// Lanes that were updated (in an interleaved event) during this render.
297
-let workInProgressRootUpdatedLanes: Lanes = NoLanes;
297
+let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes;
298
+// Lanes that were updated during the render phase (*not* an interleaved event).
299
+let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes;
300
// Lanes that were pinged (in an interleaved event) during this render.
301
let workInProgressRootPingedLanes: Lanes = NoLanes;
302
@@ -454,86 +456,105 @@ export function scheduleUpdateOnFiber(
456
eventTime: number,
457
): FiberRoot | null {
458
checkForNestedUpdates();
457
- warnAboutRenderPhaseUpdatesInDEV(fiber);
459
460
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
461
if (root === null) {
462
return null;
463
}
464
464
- if (enableUpdaterTracking) {
465
- if (isDevToolsPresent) {
466
- addFiberToLanesMap(root, fiber, lane);
467
- }
468
- }
469
-
465
// Mark that the root has a pending update.
466
markRootUpdated(root, lane, eventTime);
467
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);
468
+ if (
469
+ (executionContext & RenderContext) !== NoLanes &&
470
+ root === workInProgressRoot
471
+ ) {
472
+ // This update was dispatched during the render phase. This is a mistake
473
+ // if the update originates from user space (with the exception of local
474
+ // hook updates, which are handled differently and don't reach this
475
+ // function), but there are some internal React features that use this as
476
+ // an implementation detail, like selective hydration
477
+ // and useOpaqueIdentifier.
478
+ warnAboutRenderPhaseUpdatesInDEV(fiber);
479
+
480
+ // Track lanes that were updated during the render phase
481
+ workInProgressRootRenderPhaseUpdatedLanes = mergeLanes(
482
+ workInProgressRootRenderPhaseUpdatedLanes,
483
+ lane,
484
+ );
485
+ } else {
486
+ // This is a normal update, scheduled from outside the render phase. For
487
+ // example, during an input event.
488
+ if (enableUpdaterTracking) {
489
+ if (isDevToolsPresent) {
490
+ addFiberToLanesMap(root, fiber, lane);
491
+ }
492
+ }
493
+
494
+ if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) {
495
+ if (
496
+ (executionContext & CommitContext) !== NoContext &&
497
+ root === rootCommittingMutationOrLayoutEffects
498
+ ) {
499
+ if (fiber.mode & ProfileMode) {
500
+ let current = fiber;
501
+ while (current !== null) {
502
+ if (current.tag === Profiler) {
503
+ const {id, onNestedUpdateScheduled} = current.memoizedProps;
504
+ if (typeof onNestedUpdateScheduled === 'function') {
505
+ onNestedUpdateScheduled(id);
506
+ }
507
}
508
+ current = current.return;
509
}
487
- current = current.return;
510
}
511
}
512
}
491
- }
513
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.
514
+ // TODO: Consolidate with `isInterleavedUpdate` check
515
+ if (root === workInProgressRoot) {
516
+ // Received an update to a tree that's in the middle of rendering. Mark
517
+ // that there was an interleaved update work on this root. Unless the
518
+ // `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
519
+ // phase update. In that case, we don't treat render phase updates as if
520
+ // they were interleaved, for backwards compat reasons.
521
+ if (
522
+ deferRenderPhaseUpdateToNextBatch ||
523
+ (executionContext & RenderContext) === NoContext
524
+ ) {
525
+ workInProgressRootInterleavedUpdatedLanes = mergeLanes(
526
+ workInProgressRootInterleavedUpdatedLanes,
527
+ lane,
528
+ );
529
+ }
530
+ if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
531
+ // The root already suspended with a delay, which means this render
532
+ // definitely won't finish. Since we have a new update, let's mark it as
533
+ // suspended now, right before marking the incoming update. This has the
534
+ // effect of interrupting the current render and switching to the update.
535
+ // TODO: Make sure this doesn't override pings that happen while we've
536
+ // already started rendering.
537
+ markRootSuspended(root, workInProgressRootRenderLanes);
538
+ }
539
+ }
540
+
541
+ ensureRootIsScheduled(root, eventTime);
542
if (
501
- deferRenderPhaseUpdateToNextBatch ||
502
- (executionContext & RenderContext) === NoContext
543
+ lane === SyncLane &&
544
+ executionContext === NoContext &&
545
+ (fiber.mode & ConcurrentMode) === NoMode &&
546
+ // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
547
+ !(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
548
) {
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);
549
+ // Flush the synchronous work now, unless we're already working or inside
550
+ // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
551
+ // scheduleCallbackForFiber to preserve the ability to schedule a callback
552
+ // without immediately flushing it. We only do this for user-initiated
553
+ // updates, to preserve historical behavior of legacy mode.
554
+ resetRenderTimer();
555
+ flushSyncCallbacksOnlyInLegacyMode();
556
}
557
}
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
-
558
return root;
559
}
560
@@ -865,7 +886,25 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
886
clearContainer(root.containerInfo);
887
}
888
868
- const exitStatus = renderRootSync(root, errorRetryLanes);
889
+ let exitStatus;
890
+
891
+ const MAX_ERROR_RETRY_ATTEMPTS = 50;
892
+ for (let i = 0; i < MAX_ERROR_RETRY_ATTEMPTS; i++) {
893
+ exitStatus = renderRootSync(root, errorRetryLanes);
894
+ if (
895
+ exitStatus === RootErrored &&
896
+ workInProgressRootRenderPhaseUpdatedLanes !== NoLanes
897
+ ) {
898
+ // There was a render phase update during this render. This was likely a
899
+ // useOpaqueIdentifier hook upgrading itself to a client ID. Try rendering
900
+ // again. This time, the component will use a client ID and will proceed
901
+ // without throwing. If multiple IDs upgrade as a result of the same
902
+ // update, we will have to do multiple render passes. To protect against
903
+ // an inifinite loop, eventually we'll give up.
904
+ continue;
905
+ }
906
+ break;
907
+ }
908
909
executionContext = prevExecutionContext;
910
@@ -1042,7 +1081,10 @@ function markRootSuspended(root, suspendedLanes) {
1081
// TODO: Lol maybe there's a better way to factor this besides this
1082
// obnoxiously named function :)
1083
suspendedLanes = removeLanes(suspendedLanes, workInProgressRootPingedLanes);
1045
- suspendedLanes = removeLanes(suspendedLanes, workInProgressRootUpdatedLanes);
1084
+ suspendedLanes = removeLanes(
1085
+ suspendedLanes,
1086
+ workInProgressRootInterleavedUpdatedLanes,
1087
+ );
1088
markRootSuspended_dontCallThisOneDirectly(root, suspendedLanes);
1089
}
1090
@@ -1068,19 +1110,6 @@ function performSyncWorkOnRoot(root) {
1110
1111
let exitStatus = renderRootSync(root, lanes);
1112
if (root.tag !== LegacyRoot && exitStatus === RootErrored) {
1071
- const prevExecutionContext = executionContext;
1072
- executionContext |= RetryAfterError;
1073
-
1074
- // If an error occurred during hydration,
1075
- // discard server response and fall back to client side render.
1076
- if (root.isDehydrated) {
1077
- root.isDehydrated = false;
1078
- if (__DEV__) {
1079
- errorHydratingContainer(root.containerInfo);
1080
- }
1081
- clearContainer(root.containerInfo);
1082
- }
1083
-
1113
// If something threw an error, try rendering one more time. We'll render
1114
// synchronously to block concurrent data mutations, and we'll includes
1115
// all pending updates are included. If it still fails after the second
@@ -1088,10 +1117,8 @@ function performSyncWorkOnRoot(root) {
1117
const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root);
1118
if (errorRetryLanes !== NoLanes) {
1119
lanes = errorRetryLanes;
1091
- exitStatus = renderRootSync(root, lanes);
1120
+ exitStatus = recoverFromConcurrentError(root, errorRetryLanes);
1121
}
1093
-
1094
- executionContext = prevExecutionContext;
1122
}
1123
1124
if (exitStatus === RootFatalErrored) {
@@ -1300,7 +1327,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) {
1327
workInProgressRootExitStatus = RootIncomplete;
1328
workInProgressRootFatalError = null;
1329
workInProgressRootSkippedLanes = NoLanes;
1303
- workInProgressRootUpdatedLanes = NoLanes;
1330
+ workInProgressRootInterleavedUpdatedLanes = NoLanes;
1331
+ workInProgressRootRenderPhaseUpdatedLanes = NoLanes;
1332
workInProgressRootPingedLanes = NoLanes;
1333
1334
enqueueInterleavedUpdates();
@@ -1443,7 +1471,7 @@ export function renderDidSuspendDelayIfPossible(): void {
1471
if (
1472
workInProgressRoot !== null &&
1473
(includesNonIdleWork(workInProgressRootSkippedLanes) ||
1446
- includesNonIdleWork(workInProgressRootUpdatedLanes))
1474
+ includesNonIdleWork(workInProgressRootInterleavedUpdatedLanes))
1475
) {
1476
// Mark the current render as suspended so that we switch to working on
1477
// the updates that were skipped. Usually we only suspend at the end of
@@ -2697,7 +2725,6 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
2725
if (__DEV__) {
2726
if (
2727
ReactCurrentDebugFiberIsRenderingInDEV &&
2700
- (executionContext & RenderContext) !== NoContext &&
2728
!getIsUpdatingOpaqueValueInRenderPhaseInDEV()
2729
) {
2730
switch (fiber.tag) {
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+110
-83
@@ -294,7 +294,9 @@ let workInProgressRootIncludedLanes: Lanes = NoLanes;
294
// includes unprocessed updates, not work in bailed out children.
295
let workInProgressRootSkippedLanes: Lanes = NoLanes;
296
// Lanes that were updated (in an interleaved event) during this render.
297
-let workInProgressRootUpdatedLanes: Lanes = NoLanes;
297
+let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes;
298
+// Lanes that were updated during the render phase (*not* an interleaved event).
299
+let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes;
300
// Lanes that were pinged (in an interleaved event) during this render.
301
let workInProgressRootPingedLanes: Lanes = NoLanes;
302
@@ -454,86 +456,105 @@ export function scheduleUpdateOnFiber(
456
eventTime: number,
457
): FiberRoot | null {
458
checkForNestedUpdates();
457
- warnAboutRenderPhaseUpdatesInDEV(fiber);
459
460
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
461
if (root === null) {
462
return null;
463
}
464
464
- if (enableUpdaterTracking) {
465
- if (isDevToolsPresent) {
466
- addFiberToLanesMap(root, fiber, lane);
467
- }
468
- }
469
-
465
// Mark that the root has a pending update.
466
markRootUpdated(root, lane, eventTime);
467
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);
468
+ if (
469
+ (executionContext & RenderContext) !== NoLanes &&
470
+ root === workInProgressRoot
471
+ ) {
472
+ // This update was dispatched during the render phase. This is a mistake
473
+ // if the update originates from user space (with the exception of local
474
+ // hook updates, which are handled differently and don't reach this
475
+ // function), but there are some internal React features that use this as
476
+ // an implementation detail, like selective hydration
477
+ // and useOpaqueIdentifier.
478
+ warnAboutRenderPhaseUpdatesInDEV(fiber);
479
+
480
+ // Track lanes that were updated during the render phase
481
+ workInProgressRootRenderPhaseUpdatedLanes = mergeLanes(
482
+ workInProgressRootRenderPhaseUpdatedLanes,
483
+ lane,
484
+ );
485
+ } else {
486
+ // This is a normal update, scheduled from outside the render phase. For
487
+ // example, during an input event.
488
+ if (enableUpdaterTracking) {
489
+ if (isDevToolsPresent) {
490
+ addFiberToLanesMap(root, fiber, lane);
491
+ }
492
+ }
493
+
494
+ if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) {
495
+ if (
496
+ (executionContext & CommitContext) !== NoContext &&
497
+ root === rootCommittingMutationOrLayoutEffects
498
+ ) {
499
+ if (fiber.mode & ProfileMode) {
500
+ let current = fiber;
501
+ while (current !== null) {
502
+ if (current.tag === Profiler) {
503
+ const {id, onNestedUpdateScheduled} = current.memoizedProps;
504
+ if (typeof onNestedUpdateScheduled === 'function') {
505
+ onNestedUpdateScheduled(id);
506
+ }
507
}
508
+ current = current.return;
509
}
487
- current = current.return;
510
}
511
}
512
}
491
- }
513
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.
514
+ // TODO: Consolidate with `isInterleavedUpdate` check
515
+ if (root === workInProgressRoot) {
516
+ // Received an update to a tree that's in the middle of rendering. Mark
517
+ // that there was an interleaved update work on this root. Unless the
518
+ // `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
519
+ // phase update. In that case, we don't treat render phase updates as if
520
+ // they were interleaved, for backwards compat reasons.
521
+ if (
522
+ deferRenderPhaseUpdateToNextBatch ||
523
+ (executionContext & RenderContext) === NoContext
524
+ ) {
525
+ workInProgressRootInterleavedUpdatedLanes = mergeLanes(
526
+ workInProgressRootInterleavedUpdatedLanes,
527
+ lane,
528
+ );
529
+ }
530
+ if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
531
+ // The root already suspended with a delay, which means this render
532
+ // definitely won't finish. Since we have a new update, let's mark it as
533
+ // suspended now, right before marking the incoming update. This has the
534
+ // effect of interrupting the current render and switching to the update.
535
+ // TODO: Make sure this doesn't override pings that happen while we've
536
+ // already started rendering.
537
+ markRootSuspended(root, workInProgressRootRenderLanes);
538
+ }
539
+ }
540
+
541
+ ensureRootIsScheduled(root, eventTime);
542
if (
501
- deferRenderPhaseUpdateToNextBatch ||
502
- (executionContext & RenderContext) === NoContext
543
+ lane === SyncLane &&
544
+ executionContext === NoContext &&
545
+ (fiber.mode & ConcurrentMode) === NoMode &&
546
+ // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
547
+ !(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
548
) {
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);
549
+ // Flush the synchronous work now, unless we're already working or inside
550
+ // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
551
+ // scheduleCallbackForFiber to preserve the ability to schedule a callback
552
+ // without immediately flushing it. We only do this for user-initiated
553
+ // updates, to preserve historical behavior of legacy mode.
554
+ resetRenderTimer();
555
+ flushSyncCallbacksOnlyInLegacyMode();
556
}
557
}
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
-
558
return root;
559
}
560
@@ -865,7 +886,25 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
886
clearContainer(root.containerInfo);
887
}
888
868
- const exitStatus = renderRootSync(root, errorRetryLanes);
889
+ let exitStatus;
890
+
891
+ const MAX_ERROR_RETRY_ATTEMPTS = 50;
892
+ for (let i = 0; i < MAX_ERROR_RETRY_ATTEMPTS; i++) {
893
+ exitStatus = renderRootSync(root, errorRetryLanes);
894
+ if (
895
+ exitStatus === RootErrored &&
896
+ workInProgressRootRenderPhaseUpdatedLanes !== NoLanes
897
+ ) {
898
+ // There was a render phase update during this render. This was likely a
899
+ // useOpaqueIdentifier hook upgrading itself to a client ID. Try rendering
900
+ // again. This time, the component will use a client ID and will proceed
901
+ // without throwing. If multiple IDs upgrade as a result of the same
902
+ // update, we will have to do multiple render passes. To protect against
903
+ // an inifinite loop, eventually we'll give up.
904
+ continue;
905
+ }
906
+ break;
907
+ }
908
909
executionContext = prevExecutionContext;
910
@@ -1042,7 +1081,10 @@ function markRootSuspended(root, suspendedLanes) {
1081
// TODO: Lol maybe there's a better way to factor this besides this
1082
// obnoxiously named function :)
1083
suspendedLanes = removeLanes(suspendedLanes, workInProgressRootPingedLanes);
1045
- suspendedLanes = removeLanes(suspendedLanes, workInProgressRootUpdatedLanes);
1084
+ suspendedLanes = removeLanes(
1085
+ suspendedLanes,
1086
+ workInProgressRootInterleavedUpdatedLanes,
1087
+ );
1088
markRootSuspended_dontCallThisOneDirectly(root, suspendedLanes);
1089
}
1090
@@ -1068,19 +1110,6 @@ function performSyncWorkOnRoot(root) {
1110
1111
let exitStatus = renderRootSync(root, lanes);
1112
if (root.tag !== LegacyRoot && exitStatus === RootErrored) {
1071
- const prevExecutionContext = executionContext;
1072
- executionContext |= RetryAfterError;
1073
-
1074
- // If an error occurred during hydration,
1075
- // discard server response and fall back to client side render.
1076
- if (root.isDehydrated) {
1077
- root.isDehydrated = false;
1078
- if (__DEV__) {
1079
- errorHydratingContainer(root.containerInfo);
1080
- }
1081
- clearContainer(root.containerInfo);
1082
- }
1083
-
1113
// If something threw an error, try rendering one more time. We'll render
1114
// synchronously to block concurrent data mutations, and we'll includes
1115
// all pending updates are included. If it still fails after the second
@@ -1088,10 +1117,8 @@ function performSyncWorkOnRoot(root) {
1117
const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root);
1118
if (errorRetryLanes !== NoLanes) {
1119
lanes = errorRetryLanes;
1091
- exitStatus = renderRootSync(root, lanes);
1120
+ exitStatus = recoverFromConcurrentError(root, errorRetryLanes);
1121
}
1093
-
1094
- executionContext = prevExecutionContext;
1122
}
1123
1124
if (exitStatus === RootFatalErrored) {
@@ -1300,7 +1327,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) {
1327
workInProgressRootExitStatus = RootIncomplete;
1328
workInProgressRootFatalError = null;
1329
workInProgressRootSkippedLanes = NoLanes;
1303
- workInProgressRootUpdatedLanes = NoLanes;
1330
+ workInProgressRootInterleavedUpdatedLanes = NoLanes;
1331
+ workInProgressRootRenderPhaseUpdatedLanes = NoLanes;
1332
workInProgressRootPingedLanes = NoLanes;
1333
1334
enqueueInterleavedUpdates();
@@ -1443,7 +1471,7 @@ export function renderDidSuspendDelayIfPossible(): void {
1471
if (
1472
workInProgressRoot !== null &&
1473
(includesNonIdleWork(workInProgressRootSkippedLanes) ||
1446
- includesNonIdleWork(workInProgressRootUpdatedLanes))
1474
+ includesNonIdleWork(workInProgressRootInterleavedUpdatedLanes))
1475
) {
1476
// Mark the current render as suspended so that we switch to working on
1477
// the updates that were skipped. Usually we only suspend at the end of
@@ -2697,7 +2725,6 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
2725
if (__DEV__) {
2726
if (
2727
ReactCurrentDebugFiberIsRenderingInDEV &&
2700
- (executionContext & RenderContext) !== NoContext &&
2728
!getIsUpdatingOpaqueValueInRenderPhaseInDEV()
2729
) {
2730
switch (fiber.tag) {
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
+59
@@ -1915,6 +1915,65 @@ describe('ReactIncrementalErrorHandling', () => {
1915
expect(root).toMatchRenderedOutput('Everything is fine.');
1916
});
1917
1918
+ it("does not infinite loop if there's a render phase update in the same render as an error", async () => {
1919
+ // useOpaqueIdentifier uses an render phase update as an implementation
1920
+ // detail. When an error is accompanied by a render phase update, we assume
1921
+ // that it comes from useOpaqueIdentifier, because render phase updates
1922
+ // triggered from userspace are not allowed (we log a warning). So we keep
1923
+ // attempting to recover until no more opaque identifiers need to be
1924
+ // upgraded. However, we should give up after some point to prevent an
1925
+ // infinite loop in the case where there is (by accident) a render phase
1926
+ // triggered from userspace.
1927
+
1928
+ spyOnDev(console, 'error');
1929
+
1930
+ let numberOfThrows = 0;
1931
+
1932
+ let setStateInRenderPhase;
1933
+ function Child() {
1934
+ const [, setState] = React.useState(0);
1935
+ setStateInRenderPhase = setState;
1936
+ return 'All good';
1937
+ }
1938
+
1939
+ function App({shouldThrow}) {
1940
+ if (shouldThrow) {
1941
+ setStateInRenderPhase();
1942
+ numberOfThrows++;
1943
+ throw new Error('Oops!');
1944
+ }
1945
+ return <Child />;
1946
+ }
1947
+
1948
+ const root = ReactNoop.createRoot();
1949
+ await act(async () => {
1950
+ root.render(<App shouldThrow={false} />);
1951
+ });
1952
+ expect(root).toMatchRenderedOutput('All good');
1953
+
1954
+ let error;
1955
+ try {
1956
+ await act(async () => {
1957
+ root.render(<App shouldThrow={true} />);
1958
+ });
1959
+ } catch (e) {
1960
+ error = e;
1961
+ }
1962
+
1963
+ expect(error.message).toBe('Oops!');
1964
+ expect(numberOfThrows < 100).toBe(true);
1965
+
1966
+ if (__DEV__) {
1967
+ expect(console.error).toHaveBeenCalledTimes(2);
1968
+ expect(console.error.calls.argsFor(0)[0]).toContain(
1969
+ 'Cannot update a component (`%s`) while rendering a different component',
1970
+ );
1971
+ expect(console.error.calls.argsFor(1)[0]).toContain(
1972
+ 'The above error occurred in the <App> component',
1973
+ );
1974
+ }
1975
+ });
1976
+
1977
if (global.__PERSISTENT__) {
1978
it('regression test: should fatal if error is thrown at the root', () => {
1979
const root = ReactNoop.createRoot();