Skip to content

Commit 62e6c46

Browse files
authored
Move Mutation/Persistence fork inline into the functions (#26206)
We should never use any logic beyond declarations in the module scope, including conditions, because in a cycle that can lead to problems. More importantly, the compiler can't safely reorder statements between these points which limits the optimizations we can do.
1 parent 80cf4a0 commit 62e6c46

File tree

1 file changed

+76
-134
lines changed

1 file changed

+76
-134
lines changed

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 76 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -204,24 +204,13 @@ function hadNoMutationsEffects(current: null | Fiber, completedWork: Fiber) {
204204
return true;
205205
}
206206

207-
let appendAllChildren: (
207+
function appendAllChildren(
208208
parent: Instance,
209209
workInProgress: Fiber,
210210
needsVisibilityToggle: boolean,
211211
isHidden: boolean,
212-
) => void;
213-
let updateHostContainer;
214-
let updateHostComponent;
215-
let updateHostText;
216-
if (supportsMutation) {
217-
// Mutation mode
218-
219-
appendAllChildren = function (
220-
parent: Instance,
221-
workInProgress: Fiber,
222-
needsVisibilityToggle: boolean,
223-
isHidden: boolean,
224-
) {
212+
) {
213+
if (supportsMutation) {
225214
// We only have the top Fiber that was created but we need recurse down its
226215
// children to find all the terminal nodes.
227216
let node = workInProgress.child;
@@ -258,73 +247,7 @@ if (supportsMutation) {
258247
node.sibling.return = node.return;
259248
node = node.sibling;
260249
}
261-
};
262-
263-
updateHostContainer = function (
264-
current: null | Fiber,
265-
workInProgress: Fiber,
266-
) {
267-
// Noop
268-
};
269-
updateHostComponent = function (
270-
current: Fiber,
271-
workInProgress: Fiber,
272-
type: Type,
273-
newProps: Props,
274-
) {
275-
// If we have an alternate, that means this is an update and we need to
276-
// schedule a side-effect to do the updates.
277-
const oldProps = current.memoizedProps;
278-
if (oldProps === newProps) {
279-
// In mutation mode, this is sufficient for a bailout because
280-
// we won't touch this node even if children changed.
281-
return;
282-
}
283-
284-
// If we get updated because one of our children updated, we don't
285-
// have newProps so we'll have to reuse them.
286-
// TODO: Split the update API as separate for the props vs. children.
287-
// Even better would be if children weren't special cased at all tho.
288-
const instance: Instance = workInProgress.stateNode;
289-
const currentHostContext = getHostContext();
290-
// TODO: Experiencing an error where oldProps is null. Suggests a host
291-
// component is hitting the resume path. Figure out why. Possibly
292-
// related to `hidden`.
293-
const updatePayload = prepareUpdate(
294-
instance,
295-
type,
296-
oldProps,
297-
newProps,
298-
currentHostContext,
299-
);
300-
// TODO: Type this specific to this type of component.
301-
workInProgress.updateQueue = (updatePayload: any);
302-
// If the update payload indicates that there is a change or if there
303-
// is a new ref we mark this as an update. All the work is done in commitWork.
304-
if (updatePayload) {
305-
markUpdate(workInProgress);
306-
}
307-
};
308-
updateHostText = function (
309-
current: Fiber,
310-
workInProgress: Fiber,
311-
oldText: string,
312-
newText: string,
313-
) {
314-
// If the text differs, mark it as an update. All the work in done in commitWork.
315-
if (oldText !== newText) {
316-
markUpdate(workInProgress);
317-
}
318-
};
319-
} else if (supportsPersistence) {
320-
// Persistent host tree mode
321-
322-
appendAllChildren = function (
323-
parent: Instance,
324-
workInProgress: Fiber,
325-
needsVisibilityToggle: boolean,
326-
isHidden: boolean,
327-
) {
250+
} else if (supportsPersistence) {
328251
// We only have the top Fiber that was created but we need recurse down its
329252
// children to find all the terminal nodes.
330253
let node = workInProgress.child;
@@ -383,15 +306,17 @@ if (supportsMutation) {
383306
node.sibling.return = node.return;
384307
node = node.sibling;
385308
}
386-
};
387-
388-
// An unfortunate fork of appendAllChildren because we have two different parent types.
389-
const appendAllChildrenToContainer = function (
390-
containerChildSet: ChildSet,
391-
workInProgress: Fiber,
392-
needsVisibilityToggle: boolean,
393-
isHidden: boolean,
394-
) {
309+
}
310+
}
311+
312+
// An unfortunate fork of appendAllChildren because we have two different parent types.
313+
function appendAllChildrenToContainer(
314+
containerChildSet: ChildSet,
315+
workInProgress: Fiber,
316+
needsVisibilityToggle: boolean,
317+
isHidden: boolean,
318+
) {
319+
if (supportsPersistence) {
395320
// We only have the top Fiber that was created but we need recurse down its
396321
// children to find all the terminal nodes.
397322
let node = workInProgress.child;
@@ -457,11 +382,10 @@ if (supportsMutation) {
457382
node.sibling.return = node.return;
458383
node = node.sibling;
459384
}
460-
};
461-
updateHostContainer = function (
462-
current: null | Fiber,
463-
workInProgress: Fiber,
464-
) {
385+
}
386+
}
387+
function updateHostContainer(current: null | Fiber, workInProgress: Fiber) {
388+
if (supportsPersistence) {
465389
const portalOrRoot: {
466390
containerInfo: Container,
467391
pendingChildren: ChildSet,
@@ -480,13 +404,48 @@ if (supportsMutation) {
480404
markUpdate(workInProgress);
481405
finalizeContainerChildren(container, newChildSet);
482406
}
483-
};
484-
updateHostComponent = function (
485-
current: Fiber,
486-
workInProgress: Fiber,
487-
type: Type,
488-
newProps: Props,
489-
) {
407+
}
408+
}
409+
function updateHostComponent(
410+
current: Fiber,
411+
workInProgress: Fiber,
412+
type: Type,
413+
newProps: Props,
414+
) {
415+
if (supportsMutation) {
416+
// If we have an alternate, that means this is an update and we need to
417+
// schedule a side-effect to do the updates.
418+
const oldProps = current.memoizedProps;
419+
if (oldProps === newProps) {
420+
// In mutation mode, this is sufficient for a bailout because
421+
// we won't touch this node even if children changed.
422+
return;
423+
}
424+
425+
// If we get updated because one of our children updated, we don't
426+
// have newProps so we'll have to reuse them.
427+
// TODO: Split the update API as separate for the props vs. children.
428+
// Even better would be if children weren't special cased at all tho.
429+
const instance: Instance = workInProgress.stateNode;
430+
const currentHostContext = getHostContext();
431+
// TODO: Experiencing an error where oldProps is null. Suggests a host
432+
// component is hitting the resume path. Figure out why. Possibly
433+
// related to `hidden`.
434+
const updatePayload = prepareUpdate(
435+
instance,
436+
type,
437+
oldProps,
438+
newProps,
439+
currentHostContext,
440+
);
441+
// TODO: Type this specific to this type of component.
442+
workInProgress.updateQueue = (updatePayload: any);
443+
// If the update payload indicates that there is a change or if there
444+
// is a new ref we mark this as an update. All the work is done in commitWork.
445+
if (updatePayload) {
446+
markUpdate(workInProgress);
447+
}
448+
} else if (supportsPersistence) {
490449
const currentInstance = current.stateNode;
491450
const oldProps = current.memoizedProps;
492451
// If there are no effects associated with this node, then none of our children had any updates.
@@ -541,13 +500,20 @@ if (supportsMutation) {
541500
// If children might have changed, we have to add them all to the set.
542501
appendAllChildren(newInstance, workInProgress, false, false);
543502
}
544-
};
545-
updateHostText = function (
546-
current: Fiber,
547-
workInProgress: Fiber,
548-
oldText: string,
549-
newText: string,
550-
) {
503+
}
504+
}
505+
function updateHostText(
506+
current: Fiber,
507+
workInProgress: Fiber,
508+
oldText: string,
509+
newText: string,
510+
) {
511+
if (supportsMutation) {
512+
// If the text differs, mark it as an update. All the work in done in commitWork.
513+
if (oldText !== newText) {
514+
markUpdate(workInProgress);
515+
}
516+
} else if (supportsPersistence) {
551517
if (oldText !== newText) {
552518
// If the text content differs, we'll create a new text instance for it.
553519
const rootContainerInstance = getRootHostContainer();
@@ -564,31 +530,7 @@ if (supportsMutation) {
564530
} else {
565531
workInProgress.stateNode = current.stateNode;
566532
}
567-
};
568-
} else {
569-
// No host operations
570-
updateHostContainer = function (
571-
current: null | Fiber,
572-
workInProgress: Fiber,
573-
) {
574-
// Noop
575-
};
576-
updateHostComponent = function (
577-
current: Fiber,
578-
workInProgress: Fiber,
579-
type: Type,
580-
newProps: Props,
581-
) {
582-
// Noop
583-
};
584-
updateHostText = function (
585-
current: Fiber,
586-
workInProgress: Fiber,
587-
oldText: string,
588-
newText: string,
589-
) {
590-
// Noop
591-
};
533+
}
592534
}
593535

594536
function cutOffTailIfNeeded(

0 commit comments

Comments
 (0)