Skip to content

Commit

Permalink
Effects list rewrite
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Jul 6, 2020
1 parent 1cbaf48 commit 2d01fb4
Show file tree
Hide file tree
Showing 10 changed files with 381 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,14 @@ describe('ReactDOMServerPartialHydration', () => {
const span2 = container.getElementsByTagName('span')[0];
// This is a new node.
expect(span).not.toBe(span2);
expect(ref.current).toBe(span2);

if (gate(flags => flags.new)) {
// The effects list refactor causes this to be null because the Suspense Offscreen's child
// is null. However, since we can't hydrate Suspense in legacy this change in behavior is ok
expect(ref.current).toBe(null);
} else {
expect(ref.current).toBe(span2);
}

// Resolving the promise should render the final content.
suspend = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,12 @@ describe('createReactNativeComponentClass', () => {

expect(Text).not.toBe(View);

ReactNative.render(<Text />, 1);
ReactNative.render(<View />, 1);
ReactNative.render(
<View>
<Text />
</View>,
1,
);
});

it('should not allow viewConfigs with duplicate uiViewClassNames to be registered', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,15 @@ function ChildReconciler(shouldTrackSideEffects) {
// deletions, so we can just append the deletion to the list. The remaining
// effects aren't added until the complete phase. Once we implement
// resuming, this may not be true.
// TODO (effects) Get rid of effects list update here.
const last = returnFiber.lastEffect;
if (last !== null) {
last.nextEffect = childToDelete;
returnFiber.lastEffect = childToDelete;
} else {
returnFiber.firstEffect = returnFiber.lastEffect = childToDelete;
}
returnFiber.deletions.push(childToDelete);
childToDelete.nextEffect = null;
childToDelete.effectTag = Deletion;
}
Expand Down
6 changes: 6 additions & 0 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ function FiberNode(

// Effects
this.effectTag = NoEffect;
this.subtreeTag = NoEffect;
this.deletions = [];
this.nextEffect = null;

this.firstEffect = null;
Expand Down Expand Up @@ -287,6 +289,8 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
// We already have an alternate.
// Reset the effect tag.
workInProgress.effectTag = NoEffect;
workInProgress.subtreeTag = NoEffect;
workInProgress.deletions = [];

// The effect list is no longer valid.
workInProgress.nextEffect = null;
Expand Down Expand Up @@ -826,6 +830,8 @@ export function assignFiberPropertiesInDEV(
target.dependencies = source.dependencies;
target.mode = source.mode;
target.effectTag = source.effectTag;
target.subtreeTag = source.subtreeTag;
target.deletions = source.deletions;
target.nextEffect = source.nextEffect;
target.firstEffect = source.firstEffect;
target.lastEffect = source.lastEffect;
Expand Down
4 changes: 4 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2066,6 +2066,7 @@ function updateSuspensePrimaryChildren(
currentFallbackChildFragment.nextEffect = null;
currentFallbackChildFragment.effectTag = Deletion;
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
workInProgress.deletions.push(currentFallbackChildFragment);
}

workInProgress.child = primaryChildFragment;
Expand Down Expand Up @@ -2131,9 +2132,11 @@ function updateSuspenseFallbackChildren(
workInProgress.firstEffect = primaryChildFragment.firstEffect;
workInProgress.lastEffect = progressedLastEffect;
progressedLastEffect.nextEffect = null;
workInProgress.deletions = [];
} else {
// TODO: Reset this somewhere else? Lol legacy mode is so weird.
workInProgress.firstEffect = workInProgress.lastEffect = null;
workInProgress.deletions = [];
}
} else {
primaryChildFragment = createWorkInProgressOffscreenFiber(
Expand Down Expand Up @@ -3040,6 +3043,7 @@ function remountFiber(
} else {
returnFiber.firstEffect = returnFiber.lastEffect = current;
}
returnFiber.deletions.push(current);
current.nextEffect = null;
current.effectTag = Deletion;

Expand Down
9 changes: 9 additions & 0 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,15 @@ function completeWork(
// Reset the effect list before doing the second pass since that's now invalid.
if (renderState.lastEffect === null) {
workInProgress.firstEffect = null;
workInProgress.subtreeTag = NoEffect;
// TODO (effects) This probably isn't the best approach. Discuss with Brian
let child = workInProgress.child;
while (child !== null) {
if (child.deletions.length > 0) {
child.deletions = [];
}
child = child.sibling;
}
}
workInProgress.lastEffect = renderState.lastEffect;
// Reset the child fibers to their original state.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
HostRoot,
SuspenseComponent,
} from './ReactWorkTags';
import {Deletion, Placement, Hydrating} from './ReactSideEffectTags';
import {Deletion, Hydrating, Placement} from './ReactSideEffectTags';
import invariant from 'shared/invariant';

import {
Expand Down Expand Up @@ -125,6 +125,7 @@ function deleteHydratableInstance(
childToDelete.stateNode = instance;
childToDelete.return = returnFiber;
childToDelete.effectTag = Deletion;
returnFiber.deletions.push(childToDelete);

// This might seem like it belongs on progressedFirstDeletion. However,
// these children are not part of the reconciliation list of children.
Expand Down
Loading

0 comments on commit 2d01fb4

Please sign in to comment.