Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/renderers/shared/fiber/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
// boolean, undefined, etc.
break;
}
if (shouldTrackSideEffects) {
if (oldFiber && !newFiber.alternate) {
// We matched the slot, but we didn't reuse the existing fiber, so we
// need to delete the existing child.
deleteChild(returnFiber, oldFiber);
}
}
lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
if (!previousNewFiber) {
// TODO: Move out of the loop. This only happens for the first run.
Expand Down Expand Up @@ -573,7 +580,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
// it from the child list so that we don't add it to the deletion
// list.
existingChildren.delete(
newFiber.key === null ? newFiber.index : newFiber.key
newFiber.key === null ? newIdx : newFiber.key
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newFiber.index will be 0 if this is a clone. The index we actually matched on is oldFiber.index which also happens to be the same as newIdx which is what we just looked up in the map.

);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,124 @@ describe('ReactIncrementalSideEffects', () => {

});

it('can delete a child that changes type - implicit keys', function() {

let unmounted = false;

class ClassComponent extends React.Component {
componentWillUnmount() {
unmounted = true;
}
render() {
return <span prop="Class" />;
}
}

function FunctionalComponent(props) {
return <span prop="Function" />;
}

function Foo(props) {
return (
<div>
{props.useClass ?
<ClassComponent /> :
props.useFunction ?
<FunctionalComponent /> :
props.useText ?
'Text' :
null
}
Trail
</div>
);
}

ReactNoop.render(<Foo useClass={true} />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div(span('Class'), 'Trail'),
]);

expect(unmounted).toBe(false);

ReactNoop.render(<Foo useFunction={true} />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div(span('Function'), 'Trail'),
]);

expect(unmounted).toBe(true);

ReactNoop.render(<Foo useText={true} />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div('Text', 'Trail'),
]);

ReactNoop.render(<Foo />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div('Trail'),
]);

});

it('can delete a child that changes type - explicit keys', function() {

let unmounted = false;

class ClassComponent extends React.Component {
componentWillUnmount() {
unmounted = true;
}
render() {
return <span prop="Class" />;
}
}

function FunctionalComponent(props) {
return <span prop="Function" />;
}

function Foo(props) {
return (
<div>
{props.useClass ?
<ClassComponent key="a" /> :
props.useFunction ?
<FunctionalComponent key="a" /> :
null
}
Trail
</div>
);
}

ReactNoop.render(<Foo useClass={true} />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div(span('Class'), 'Trail'),
]);

expect(unmounted).toBe(false);

ReactNoop.render(<Foo useFunction={true} />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div(span('Function'), 'Trail'),
]);

expect(unmounted).toBe(true);

ReactNoop.render(<Foo />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div('Trail'),
]);

});

it('does not update child nodes if a flush is aborted', () => {

function Bar(props) {
Expand Down