@samitouri / QOS-React / commits / 62e6c4612e

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.

Sebastian Markbåge committed Feb 20, 2023 at 15:03 UTC 62e6c4612ec704cf07c99b318bc8f26d3ec3f588
1 file changed +76 -134
packages/react-reconciler/src/ReactFiberCompleteWork.js
+76 -134
@@ -204,24 +204,13 @@ function hadNoMutationsEffects(current: null | Fiber, completedWork: Fiber) {
204 return true;
205 }
206
207 -let appendAllChildren: (
207 +function appendAllChildren(
208 parent: Instance,
209 workInProgress: Fiber,
210 needsVisibilityToggle: boolean,
211 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) {
214 // We only have the top Fiber that was created but we need recurse down its
215 // children to find all the terminal nodes.
216 let node = workInProgress.child;
@@ -258,73 +247,7 @@ if (supportsMutation) {
247 node.sibling.return = node.return;
248 node = node.sibling;
249 }
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) {
251 // We only have the top Fiber that was created but we need recurse down its
252 // children to find all the terminal nodes.
253 let node = workInProgress.child;
@@ -383,15 +306,17 @@ if (supportsMutation) {
306 node.sibling.return = node.return;
307 node = node.sibling;
308 }
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) {
320 // We only have the top Fiber that was created but we need recurse down its
321 // children to find all the terminal nodes.
322 let node = workInProgress.child;
@@ -457,11 +382,10 @@ if (supportsMutation) {
382 node.sibling.return = node.return;
383 node = node.sibling;
384 }
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) {
389 const portalOrRoot: {
390 containerInfo: Container,
391 pendingChildren: ChildSet,
@@ -480,13 +404,48 @@ if (supportsMutation) {
404 markUpdate(workInProgress);
405 finalizeContainerChildren(container, newChildSet);
406 }
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) {
449 const currentInstance = current.stateNode;
450 const oldProps = current.memoizedProps;
451 // If there are no effects associated with this node, then none of our children had any updates.
@@ -541,13 +500,20 @@ if (supportsMutation) {
500 // If children might have changed, we have to add them all to the set.
501 appendAllChildren(newInstance, workInProgress, false, false);
502 }
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) {
517 if (oldText !== newText) {
518 // If the text content differs, we'll create a new text instance for it.
519 const rootContainerInstance = getRootHostContainer();
@@ -564,31 +530,7 @@ if (supportsMutation) {
530 } else {
531 workInProgress.stateNode = current.stateNode;
532 }
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 + }
534 }
535
536 function cutOffTailIfNeeded(