Skip to content

Commit

Permalink
Remove unnecessary try/finally blocks
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Sep 6, 2023
1 parent b9be453 commit d0d6924
Showing 1 changed file with 28 additions and 22 deletions.
50 changes: 28 additions & 22 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1021,12 +1021,13 @@ function renderIndeterminateComponent(
const prevTreeContext = task.treeContext;
const totalChildren = 1;
const index = 0;
// Modify the id context. Because we'll need to reset this if something
// suspends or errors, we'll use the non-destructive render path.
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
try {
renderNodeDestructive(request, task, null, value, 0);
} finally {
task.treeContext = prevTreeContext;
}
renderNode(request, task, value, 0);
// Like the other contexts, this does not need to be in a finally block
// because renderNode takes care of unwinding the stack.
task.treeContext = prevTreeContext;
} else {
renderNodeDestructive(request, task, null, value, 0);
}
Expand Down Expand Up @@ -1126,12 +1127,12 @@ function renderForwardRef(
const prevTreeContext = task.treeContext;
const totalChildren = 1;
const index = 0;
// Modify the id context. Because we'll need to reset this if something
// suspends or errors, we'll use the non-destructive render path.
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, index);
try {
renderNodeDestructive(request, task, null, children, 0);
} finally {
task.treeContext = prevTreeContext;
}
renderNode(request, task, children, 0);
// Like the other contexts, this does not need to be in a finally block
// because renderNode takes care of unwinding the stack.
} else {
renderNodeDestructive(request, task, null, children, 0);
}
Expand Down Expand Up @@ -1656,26 +1657,27 @@ function renderChildrenArray(
children: Array<any>,
childIndex: number,
) {
const prevKeyPath = task.keyPath;
const prevTreeContext = task.treeContext;
const totalChildren = children.length;
for (let i = 0; i < totalChildren; i++) {
const prevTreeContext = task.treeContext;
const node = children[i];
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i);
try {
const node = children[i];
if (isArray(node) || getIteratorFn(node)) {
// Nested arrays behave like a "fragment node" which is keyed.
// Therefore we need to add the current index as a parent key.
task.keyPath = [task.keyPath, '', childIndex];
}
if (isArray(node) || getIteratorFn(node)) {
// Nested arrays behave like a "fragment node" which is keyed.
// Therefore we need to add the current index as a parent key.
const prevKeyPath = task.keyPath;
task.keyPath = [task.keyPath, '', childIndex];
renderNode(request, task, node, i);
task.keyPath = prevKeyPath;
} else {
// We need to use the non-destructive form so that we can safely pop back
// up and render the sibling if something suspends.
renderNode(request, task, node, i);
} finally {
task.treeContext = prevTreeContext;
task.keyPath = prevKeyPath;
}
}
// Because this context is always set right before rendering every child, we
// only need to reset it to the previous value at the very end.
task.treeContext = prevTreeContext;
}

function trackPostpone(
Expand Down Expand Up @@ -1824,6 +1826,7 @@ function renderNode(
const previousLegacyContext = task.legacyContext;
const previousContext = task.context;
const previousKeyPath = task.keyPath;
const previousTreeContext = task.treeContext;
let previousComponentStack = null;
if (__DEV__) {
previousComponentStack = task.componentStack;
Expand Down Expand Up @@ -1860,6 +1863,7 @@ function renderNode(
task.legacyContext = previousLegacyContext;
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
// Restore all active ReactContexts to what they were before.
switchContext(previousContext);
if (__DEV__) {
Expand Down Expand Up @@ -1892,6 +1896,7 @@ function renderNode(
task.legacyContext = previousLegacyContext;
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
// Restore all active ReactContexts to what they were before.
switchContext(previousContext);
if (__DEV__) {
Expand All @@ -1906,6 +1911,7 @@ function renderNode(
task.legacyContext = previousLegacyContext;
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
// Restore all active ReactContexts to what they were before.
switchContext(previousContext);
if (__DEV__) {
Expand Down

0 comments on commit d0d6924

Please sign in to comment.