Skip to content

Commit 9cedfe5

Browse files
acdliteacdlite
committed
useId: Remove unnecessary try/finally blocks (#27340)
To generate IDs for useId, we modify a context variable whenever multiple siblings are rendered, or when a component includes a useId hook. When this happens, we must ensure that the context is reset properly on unwind if something errors or suspends. When I originally implemented this, I did this by wrapping the child's rendering with a try/finally block. But a better way to do this is by using the non-destructive renderNode path instead of renderNodeDestructive. DiffTrain build for [ee7f9c9](ee7f9c9)
1 parent 3be7229 commit 9cedfe5

8 files changed

+198
-205
lines changed

‎compiled/facebook-www/REVISION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2c2bdd0ffe54df60201ee93b29de5cb7b93ff029
1+
ee7f9c9351f8902e07ceacf4234ef75e7e4ecd73

‎compiled/facebook-www/ReactDOMServer-dev.classic.js‎

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ if (__DEV__) {
1919
var React = require("react");
2020
var ReactDOM = require("react-dom");
2121

22-
var ReactVersion = "18.3.0-www-classic-9855f7ea";
22+
var ReactVersion = "18.3.0-www-classic-84281c28";
2323

2424
// This refers to a WWW module.
2525
var warningWWW = require("warning");
@@ -10434,14 +10434,14 @@ function renderIndeterminateComponent(
1043410434
// a single "child" slot.
1043510435
var prevTreeContext = task.treeContext;
1043610436
var totalChildren = 1;
10437-
var index = 0;
10437+
var index = 0; // Modify the id context. Because we'll need to reset this if something
10438+
// suspends or errors, we'll use the non-destructive render path.
10439+
1043810440
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
10441+
renderNode(request, task, value, 0); // Like the other contexts, this does not need to be in a finally block
10442+
// because renderNode takes care of unwinding the stack.
1043910443

10440-
try {
10441-
renderNodeDestructive(request, task, null, value, 0);
10442-
} finally {
10443-
task.treeContext = prevTreeContext;
10444-
}
10444+
task.treeContext = prevTreeContext;
1044510445
} else {
1044610446
renderNodeDestructive(request, task, null, value, 0);
1044710447
}
@@ -10541,14 +10541,12 @@ function renderForwardRef(request, task, prevThenableState, type, props, ref) {
1054110541
// a single "child" slot.
1054210542
var prevTreeContext = task.treeContext;
1054310543
var totalChildren = 1;
10544-
var index = 0;
10545-
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
10544+
var index = 0; // Modify the id context. Because we'll need to reset this if something
10545+
// suspends or errors, we'll use the non-destructive render path.
1054610546

10547-
try {
10548-
renderNodeDestructive(request, task, null, children, 0);
10549-
} finally {
10550-
task.treeContext = prevTreeContext;
10551-
}
10547+
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
10548+
renderNode(request, task, children, 0); // Like the other contexts, this does not need to be in a finally block
10549+
// because renderNode takes care of unwinding the stack.
1055210550
} else {
1055310551
renderNodeDestructive(request, task, null, children, 0);
1055410552
}
@@ -11048,29 +11046,29 @@ function renderNodeDestructiveImpl(
1104811046
}
1104911047

1105011048
function renderChildrenArray(request, task, children, childIndex) {
11051-
var prevKeyPath = task.keyPath;
11049+
var prevTreeContext = task.treeContext;
1105211050
var totalChildren = children.length;
1105311051

1105411052
for (var i = 0; i < totalChildren; i++) {
11055-
var prevTreeContext = task.treeContext;
11053+
var node = children[i];
1105611054
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i);
1105711055

11058-
try {
11059-
var node = children[i];
11060-
11061-
if (isArray(node) || getIteratorFn(node)) {
11062-
// Nested arrays behave like a "fragment node" which is keyed.
11063-
// Therefore we need to add the current index as a parent key.
11064-
task.keyPath = [task.keyPath, "", childIndex];
11065-
} // We need to use the non-destructive form so that we can safely pop back
11066-
// up and render the sibling if something suspends.
11067-
11056+
if (isArray(node) || getIteratorFn(node)) {
11057+
// Nested arrays behave like a "fragment node" which is keyed.
11058+
// Therefore we need to add the current index as a parent key.
11059+
var prevKeyPath = task.keyPath;
11060+
task.keyPath = [task.keyPath, "", childIndex];
1106811061
renderNode(request, task, node, i);
11069-
} finally {
11070-
task.treeContext = prevTreeContext;
1107111062
task.keyPath = prevKeyPath;
11063+
} else {
11064+
// We need to use the non-destructive form so that we can safely pop back
11065+
// up and render the sibling if something suspends.
11066+
renderNode(request, task, node, i);
1107211067
}
11073-
}
11068+
} // Because this context is always set right before rendering every child, we
11069+
// only need to reset it to the previous value at the very end.
11070+
11071+
task.treeContext = prevTreeContext;
1107411072
}
1107511073

1107611074
function spawnNewSuspendedTask(request, task, thenableState, x) {
@@ -11127,6 +11125,7 @@ function renderNode(request, task, node, childIndex) {
1112711125
var previousLegacyContext = task.legacyContext;
1112811126
var previousContext = task.context;
1112911127
var previousKeyPath = task.keyPath;
11128+
var previousTreeContext = task.treeContext;
1113011129
var previousComponentStack = null;
1113111130

1113211131
{
@@ -11160,7 +11159,8 @@ function renderNode(request, task, node, childIndex) {
1116011159
task.formatContext = previousFormatContext;
1116111160
task.legacyContext = previousLegacyContext;
1116211161
task.context = previousContext;
11163-
task.keyPath = previousKeyPath; // Restore all active ReactContexts to what they were before.
11162+
task.keyPath = previousKeyPath;
11163+
task.treeContext = previousTreeContext; // Restore all active ReactContexts to what they were before.
1116411164

1116511165
switchContext(previousContext);
1116611166

@@ -11176,7 +11176,8 @@ function renderNode(request, task, node, childIndex) {
1117611176
task.formatContext = previousFormatContext;
1117711177
task.legacyContext = previousLegacyContext;
1117811178
task.context = previousContext;
11179-
task.keyPath = previousKeyPath; // Restore all active ReactContexts to what they were before.
11179+
task.keyPath = previousKeyPath;
11180+
task.treeContext = previousTreeContext; // Restore all active ReactContexts to what they were before.
1118011181

1118111182
switchContext(previousContext);
1118211183

‎compiled/facebook-www/ReactDOMServer-dev.modern.js‎

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ if (__DEV__) {
1919
var React = require("react");
2020
var ReactDOM = require("react-dom");
2121

22-
var ReactVersion = "18.3.0-www-modern-b63e4245";
22+
var ReactVersion = "18.3.0-www-modern-7792b590";
2323

2424
// This refers to a WWW module.
2525
var warningWWW = require("warning");
@@ -10182,14 +10182,14 @@ function renderIndeterminateComponent(
1018210182
// a single "child" slot.
1018310183
var prevTreeContext = task.treeContext;
1018410184
var totalChildren = 1;
10185-
var index = 0;
10185+
var index = 0; // Modify the id context. Because we'll need to reset this if something
10186+
// suspends or errors, we'll use the non-destructive render path.
10187+
1018610188
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
10189+
renderNode(request, task, value, 0); // Like the other contexts, this does not need to be in a finally block
10190+
// because renderNode takes care of unwinding the stack.
1018710191

10188-
try {
10189-
renderNodeDestructive(request, task, null, value, 0);
10190-
} finally {
10191-
task.treeContext = prevTreeContext;
10192-
}
10192+
task.treeContext = prevTreeContext;
1019310193
} else {
1019410194
renderNodeDestructive(request, task, null, value, 0);
1019510195
}
@@ -10289,14 +10289,12 @@ function renderForwardRef(request, task, prevThenableState, type, props, ref) {
1028910289
// a single "child" slot.
1029010290
var prevTreeContext = task.treeContext;
1029110291
var totalChildren = 1;
10292-
var index = 0;
10293-
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
10292+
var index = 0; // Modify the id context. Because we'll need to reset this if something
10293+
// suspends or errors, we'll use the non-destructive render path.
1029410294

10295-
try {
10296-
renderNodeDestructive(request, task, null, children, 0);
10297-
} finally {
10298-
task.treeContext = prevTreeContext;
10299-
}
10295+
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
10296+
renderNode(request, task, children, 0); // Like the other contexts, this does not need to be in a finally block
10297+
// because renderNode takes care of unwinding the stack.
1030010298
} else {
1030110299
renderNodeDestructive(request, task, null, children, 0);
1030210300
}
@@ -10796,29 +10794,29 @@ function renderNodeDestructiveImpl(
1079610794
}
1079710795

1079810796
function renderChildrenArray(request, task, children, childIndex) {
10799-
var prevKeyPath = task.keyPath;
10797+
var prevTreeContext = task.treeContext;
1080010798
var totalChildren = children.length;
1080110799

1080210800
for (var i = 0; i < totalChildren; i++) {
10803-
var prevTreeContext = task.treeContext;
10801+
var node = children[i];
1080410802
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i);
1080510803

10806-
try {
10807-
var node = children[i];
10808-
10809-
if (isArray(node) || getIteratorFn(node)) {
10810-
// Nested arrays behave like a "fragment node" which is keyed.
10811-
// Therefore we need to add the current index as a parent key.
10812-
task.keyPath = [task.keyPath, "", childIndex];
10813-
} // We need to use the non-destructive form so that we can safely pop back
10814-
// up and render the sibling if something suspends.
10815-
10804+
if (isArray(node) || getIteratorFn(node)) {
10805+
// Nested arrays behave like a "fragment node" which is keyed.
10806+
// Therefore we need to add the current index as a parent key.
10807+
var prevKeyPath = task.keyPath;
10808+
task.keyPath = [task.keyPath, "", childIndex];
1081610809
renderNode(request, task, node, i);
10817-
} finally {
10818-
task.treeContext = prevTreeContext;
1081910810
task.keyPath = prevKeyPath;
10811+
} else {
10812+
// We need to use the non-destructive form so that we can safely pop back
10813+
// up and render the sibling if something suspends.
10814+
renderNode(request, task, node, i);
1082010815
}
10821-
}
10816+
} // Because this context is always set right before rendering every child, we
10817+
// only need to reset it to the previous value at the very end.
10818+
10819+
task.treeContext = prevTreeContext;
1082210820
}
1082310821

1082410822
function spawnNewSuspendedTask(request, task, thenableState, x) {
@@ -10875,6 +10873,7 @@ function renderNode(request, task, node, childIndex) {
1087510873
var previousLegacyContext = task.legacyContext;
1087610874
var previousContext = task.context;
1087710875
var previousKeyPath = task.keyPath;
10876+
var previousTreeContext = task.treeContext;
1087810877
var previousComponentStack = null;
1087910878

1088010879
{
@@ -10908,7 +10907,8 @@ function renderNode(request, task, node, childIndex) {
1090810907
task.formatContext = previousFormatContext;
1090910908
task.legacyContext = previousLegacyContext;
1091010909
task.context = previousContext;
10911-
task.keyPath = previousKeyPath; // Restore all active ReactContexts to what they were before.
10910+
task.keyPath = previousKeyPath;
10911+
task.treeContext = previousTreeContext; // Restore all active ReactContexts to what they were before.
1091210912

1091310913
switchContext(previousContext);
1091410914

@@ -10924,7 +10924,8 @@ function renderNode(request, task, node, childIndex) {
1092410924
task.formatContext = previousFormatContext;
1092510925
task.legacyContext = previousLegacyContext;
1092610926
task.context = previousContext;
10927-
task.keyPath = previousKeyPath; // Restore all active ReactContexts to what they were before.
10927+
task.keyPath = previousKeyPath;
10928+
task.treeContext = previousTreeContext; // Restore all active ReactContexts to what they were before.
1092810929

1092910930
switchContext(previousContext);
1093010931

‎compiled/facebook-www/ReactDOMServer-prod.classic.js‎

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3174,24 +3174,25 @@ function renderElement(request, task, prevThenableState, type, props, ref) {
31743174
renderNodeDestructiveImpl(request, task, null, props, 0);
31753175
task.legacyContext = prevThenableState;
31763176
} else renderNodeDestructiveImpl(request, task, null, props, 0);
3177-
} else if (
3178-
((contextKey = getMaskedContext(type, task.legacyContext)),
3179-
(currentlyRenderingComponent = {}),
3180-
(currentlyRenderingTask = task),
3181-
(thenableIndexCounter = localIdCounter = 0),
3182-
(thenableState = prevThenableState),
3183-
(JSCompiler_inline_result = type(props, contextKey)),
3184-
(props = finishHooks(type, props, JSCompiler_inline_result, contextKey)),
3185-
0 !== localIdCounter)
3186-
) {
3187-
type = task.treeContext;
3188-
task.treeContext = pushTreeContext(type, 1, 0);
3189-
try {
3190-
renderNodeDestructiveImpl(request, task, null, props, 0);
3191-
} finally {
3192-
task.treeContext = type;
3193-
}
3194-
} else renderNodeDestructiveImpl(request, task, null, props, 0);
3177+
} else
3178+
(contextKey = getMaskedContext(type, task.legacyContext)),
3179+
(currentlyRenderingComponent = {}),
3180+
(currentlyRenderingTask = task),
3181+
(thenableIndexCounter = localIdCounter = 0),
3182+
(thenableState = prevThenableState),
3183+
(JSCompiler_inline_result = type(props, contextKey)),
3184+
(props = finishHooks(
3185+
type,
3186+
props,
3187+
JSCompiler_inline_result,
3188+
contextKey
3189+
)),
3190+
0 !== localIdCounter
3191+
? ((type = task.treeContext),
3192+
(task.treeContext = pushTreeContext(type, 1, 0)),
3193+
renderNode(request, task, props, 0),
3194+
(task.treeContext = type))
3195+
: renderNodeDestructiveImpl(request, task, null, props, 0);
31953196
else if ("string" === typeof type) {
31963197
contextKey = task.blockedSegment;
31973198
prevThenableState = pushStartInstance(
@@ -3365,15 +3366,10 @@ function renderElement(request, task, prevThenableState, type, props, ref) {
33653366
thenableState = prevThenableState;
33663367
contextKey = type(props, ref);
33673368
props = finishHooks(type, props, contextKey, ref);
3368-
if (0 !== localIdCounter) {
3369-
type = task.treeContext;
3370-
task.treeContext = pushTreeContext(type, 1, 0);
3371-
try {
3372-
renderNodeDestructiveImpl(request, task, null, props, 0);
3373-
} finally {
3374-
task.treeContext = type;
3375-
}
3376-
} else renderNodeDestructiveImpl(request, task, null, props, 0);
3369+
0 !== localIdCounter
3370+
? ((task.treeContext = pushTreeContext(task.treeContext, 1, 0)),
3371+
renderNode(request, task, props, 0))
3372+
: renderNodeDestructiveImpl(request, task, null, props, 0);
33773373
return;
33783374
case REACT_MEMO_TYPE:
33793375
type = type.type;
@@ -3515,21 +3511,22 @@ function renderNodeDestructiveImpl(
35153511
}
35163512
function renderChildrenArray(request, task, children, childIndex) {
35173513
for (
3518-
var prevKeyPath = task.keyPath, totalChildren = children.length, i = 0;
3514+
var prevTreeContext = task.treeContext,
3515+
totalChildren = children.length,
3516+
i = 0;
35193517
i < totalChildren;
35203518
i++
35213519
) {
3522-
var prevTreeContext = task.treeContext;
3520+
var node = children[i];
35233521
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i);
3524-
try {
3525-
var node = children[i];
3526-
if (isArrayImpl(node) || getIteratorFn(node))
3527-
task.keyPath = [task.keyPath, "", childIndex];
3522+
if (isArrayImpl(node) || getIteratorFn(node)) {
3523+
var prevKeyPath = task.keyPath;
3524+
task.keyPath = [task.keyPath, "", childIndex];
35283525
renderNode(request, task, node, i);
3529-
} finally {
3530-
(task.treeContext = prevTreeContext), (task.keyPath = prevKeyPath);
3531-
}
3526+
task.keyPath = prevKeyPath;
3527+
} else renderNode(request, task, node, i);
35323528
}
3529+
task.treeContext = prevTreeContext;
35333530
}
35343531
function renderNode(request, task, node, childIndex) {
35353532
var segment = task.blockedSegment,
@@ -3538,7 +3535,8 @@ function renderNode(request, task, node, childIndex) {
35383535
previousFormatContext = task.formatContext,
35393536
previousLegacyContext = task.legacyContext,
35403537
previousContext = task.context,
3541-
previousKeyPath = task.keyPath;
3538+
previousKeyPath = task.keyPath,
3539+
previousTreeContext = task.treeContext;
35423540
try {
35433541
return renderNodeDestructiveImpl(request, task, null, node, childIndex);
35443542
} catch (thrownValue) {
@@ -3584,13 +3582,15 @@ function renderNode(request, task, node, childIndex) {
35843582
(task.legacyContext = previousLegacyContext),
35853583
(task.context = previousContext),
35863584
(task.keyPath = previousKeyPath),
3585+
(task.treeContext = previousTreeContext),
35873586
switchContext(previousContext);
35883587
else
35893588
throw (
35903589
((task.formatContext = previousFormatContext),
35913590
(task.legacyContext = previousLegacyContext),
35923591
(task.context = previousContext),
35933592
(task.keyPath = previousKeyPath),
3593+
(task.treeContext = previousTreeContext),
35943594
switchContext(previousContext),
35953595
node)
35963596
);
@@ -4431,4 +4431,4 @@ exports.renderToString = function (children, options) {
44314431
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
44324432
);
44334433
};
4434-
exports.version = "18.3.0-www-classic-01cd1133";
4434+
exports.version = "18.3.0-www-classic-5ae5e167";

0 commit comments

Comments
 (0)