[DevTools] Remove use of .alternate in root and recordProfilingDurations (#30895)
Ideally we shouldn't use the `.alternate` to access previous state because ideally Fibers shouldn't have alternates. The only case it's ok to use it is when it is used to identity the stateful part of a component's identity. In a non-alternate Fiber model there would instead be another object that represents instance but in the current model it's modeled by the pair. It's not ok is to get the previous state of the tree since that would not live on the stateful part. We don't generally need this though because we have the previous state on instance.data before updating it, or passed from above.
Sebastian Markbåge committed
Sep 6, 2024 at 21:46 UTC
baf47462d631a1f87063676d36a33b4a3f6ed80b
1 file changed
+21
-18
packages/react-devtools-shared/src/backend/fiber/renderer.js
+21
-18
@@ -2371,7 +2371,7 @@ export function attach(
2371
}
2372
2373
if (isProfilingSupported) {
2374
- recordProfilingDurations(fiberInstance);
2374
+ recordProfilingDurations(fiberInstance, null);
2375
}
2376
return fiberInstance;
2377
}
@@ -2937,7 +2937,10 @@ export function attach(
2937
removeChild(instance, null);
2938
}
2939
2940
- function recordProfilingDurations(fiberInstance: FiberInstance) {
2940
+ function recordProfilingDurations(
2941
+ fiberInstance: FiberInstance,
2942
+ prevFiber: null | Fiber,
2943
+ ) {
2944
const id = fiberInstance.id;
2945
const fiber = fiberInstance.data;
2946
const {actualDuration, treeBaseDuration} = fiber;
@@ -2945,13 +2948,11 @@ export function attach(
2948
fiberInstance.treeBaseDuration = treeBaseDuration || 0;
2949
2950
if (isProfiling) {
2948
- const {alternate} = fiber;
2949
-
2951
// It's important to update treeBaseDuration even if the current Fiber did not render,
2952
// because it's possible that one of its descendants did.
2953
if (
2953
- alternate == null ||
2954
- treeBaseDuration !== alternate.treeBaseDuration
2954
+ prevFiber == null ||
2955
+ treeBaseDuration !== prevFiber.treeBaseDuration
2956
) {
2957
// Tree base duration updates are included in the operations typed array.
2958
// So we have to convert them from milliseconds to microseconds so we can send them as ints.
@@ -2963,7 +2964,7 @@ export function attach(
2964
pushOperation(convertedTreeBaseDuration);
2965
}
2966
2966
- if (alternate == null || didFiberRender(alternate, fiber)) {
2967
+ if (prevFiber == null || didFiberRender(prevFiber, fiber)) {
2968
if (actualDuration != null) {
2969
// The actual duration reported by React includes time spent working on children.
2970
// This is useful information, but it's also useful to be able to exclude child durations.
@@ -2991,7 +2992,7 @@ export function attach(
2992
);
2993
2994
if (recordChangeDescriptions) {
2994
- const changeDescription = getChangeDescription(alternate, fiber);
2995
+ const changeDescription = getChangeDescription(prevFiber, fiber);
2996
if (changeDescription !== null) {
2997
if (metadata.changeDescriptions !== null) {
2998
metadata.changeDescriptions.set(id, changeDescription);
@@ -3314,8 +3315,6 @@ export function attach(
3315
// Register the new alternate in case it's not already in.
3316
fiberToFiberInstanceMap.set(nextChild, fiberInstance);
3317
3317
- // Update the Fiber so we that we always keep the current Fiber on the data.
3318
- fiberInstance.data = nextChild;
3318
moveChild(fiberInstance, previousSiblingOfExistingInstance);
3319
3320
if (
@@ -3455,6 +3454,8 @@ export function attach(
3454
const stashedPrevious = previouslyReconciledSibling;
3455
const stashedRemaining = remainingReconcilingChildren;
3456
if (fiberInstance !== null) {
3457
+ // Update the Fiber so we that we always keep the current Fiber on the data.
3458
+ fiberInstance.data = nextFiber;
3459
if (
3460
mostRecentlyInspectedElement !== null &&
3461
mostRecentlyInspectedElement.id === fiberInstance.id &&
@@ -3610,7 +3611,7 @@ export function attach(
3611
const isProfilingSupported =
3612
nextFiber.hasOwnProperty('treeBaseDuration');
3613
if (isProfilingSupported) {
3613
- recordProfilingDurations(fiberInstance);
3614
+ recordProfilingDurations(fiberInstance, prevFiber);
3615
}
3616
}
3617
if (shouldResetChildren) {
@@ -3681,11 +3682,11 @@ export function attach(
3682
// If we have not been profiling, then we can just walk the tree and build up its current state as-is.
3683
hook.getFiberRoots(rendererID).forEach(root => {
3684
const current = root.current;
3684
- const alternate = current.alternate;
3685
const newRoot = createFiberInstance(current);
3686
rootToFiberInstanceMap.set(root, newRoot);
3687
idToDevToolsInstanceMap.set(newRoot.id, newRoot);
3688
fiberToFiberInstanceMap.set(current, newRoot);
3689
+ const alternate = current.alternate;
3690
if (alternate) {
3691
fiberToFiberInstanceMap.set(alternate, newRoot);
3692
}
@@ -3755,20 +3756,22 @@ export function attach(
3756
priorityLevel: void | number,
3757
) {
3758
const current = root.current;
3758
- const alternate = current.alternate;
3759
3760
+ let prevFiber: null | Fiber = null;
3761
let rootInstance = rootToFiberInstanceMap.get(root);
3762
if (!rootInstance) {
3763
rootInstance = createFiberInstance(current);
3764
rootToFiberInstanceMap.set(root, rootInstance);
3765
idToDevToolsInstanceMap.set(rootInstance.id, rootInstance);
3766
fiberToFiberInstanceMap.set(current, rootInstance);
3767
+ const alternate = current.alternate;
3768
if (alternate) {
3769
fiberToFiberInstanceMap.set(alternate, rootInstance);
3770
}
3771
currentRootID = rootInstance.id;
3772
} else {
3773
currentRootID = rootInstance.id;
3774
+ prevFiber = rootInstance.data;
3775
}
3776
3777
// Before the traversals, remember to start tracking
@@ -3804,13 +3807,13 @@ export function attach(
3807
};
3808
}
3809
3807
- if (alternate) {
3810
+ if (prevFiber !== null) {
3811
// TODO: relying on this seems a bit fishy.
3812
const wasMounted =
3810
- alternate.memoizedState != null &&
3811
- alternate.memoizedState.element != null &&
3813
+ prevFiber.memoizedState != null &&
3814
+ prevFiber.memoizedState.element != null &&
3815
// A dehydrated root is not considered mounted
3813
- alternate.memoizedState.isDehydrated !== true;
3816
+ prevFiber.memoizedState.isDehydrated !== true;
3817
const isMounted =
3818
current.memoizedState != null &&
3819
current.memoizedState.element != null &&
@@ -3822,7 +3825,7 @@ export function attach(
3825
mountFiberRecursively(current, false);
3826
} else if (wasMounted && isMounted) {
3827
// Update an existing root.
3825
- updateFiberRecursively(rootInstance, current, alternate, false);
3828
+ updateFiberRecursively(rootInstance, current, prevFiber, false);
3829
} else if (wasMounted && !isMounted) {
3830
// Unmount an existing root.
3831
unmountInstanceRecursively(rootInstance);