Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d9393ee
Add Fragment fiber type
sebmarkbage Sep 13, 2016
e8c1cb4
Add Text node types
sebmarkbage Sep 14, 2016
d7322d8
Silence Fiber warning when the feature flag is on
sebmarkbage Sep 13, 2016
e05ab67
Fix MultiChild tests so they work with Fiber
sebmarkbage Sep 13, 2016
8c7409b
Add comment about bug in yields
sebmarkbage Sep 15, 2016
4ed67f1
Enable text updates in ReactNoop
sebmarkbage Sep 19, 2016
dff0faf
Fiber child reconciliation
sebmarkbage Sep 20, 2016
51f2bf9
Add index field to each fiber
sebmarkbage Sep 21, 2016
bcbceae
Don't track side-effects unless needed
sebmarkbage Sep 21, 2016
c49a91c
Fast path for create child
sebmarkbage Oct 4, 2016
be0acf8
Deletion tracking
sebmarkbage Oct 4, 2016
76725fe
Tag the fiber with the kind of side-effect that was applied to it
sebmarkbage Oct 4, 2016
86e854e
Append deletions to the effect list
sebmarkbage Oct 4, 2016
eabed69
Move child updates to use the reconciled effects
sebmarkbage Oct 5, 2016
31ca1e7
Remove beginWork shortcut
sebmarkbage Oct 6, 2016
0262e70
Reset effect list when we recompute children
sebmarkbage Oct 7, 2016
f3d7116
Always override priority level when visiting children
sebmarkbage Oct 7, 2016
40989f8
Call componentWillUnmount during deletion phase
sebmarkbage Oct 7, 2016
8360088
Fire componentDidMount/componentDidUpdate life-cycles
sebmarkbage Oct 7, 2016
48cf81c
Resolve ref callbacks
sebmarkbage Oct 7, 2016
8721ec1
Invoke all null ref calls before any new ref calls
sebmarkbage Oct 7, 2016
d9efde7
Fix resuming bug
sebmarkbage Oct 10, 2016
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
Prev Previous commit
Next Next commit
Call componentWillUnmount during deletion phase
While we're deleting nodes, we need to call the unmount
life-cycle. However, there is a special case that we don't want
to keep deleting every host node along the way since deleting the
top node is enough.
  • Loading branch information
sebmarkbage committed Oct 17, 2016
commit 40989f8d7000965cc6a07bb795d59468f23a2195
53 changes: 49 additions & 4 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,36 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
}
}

function commitNestedUnmounts(root : Fiber) {
// While we're inside a removed host node we don't want to call
// removeChild on the inner nodes because they're removed by the top
// call anyway. We also want to call componentWillUnmount on all
// composites before this host node is removed from the tree. Therefore
// we do an inner loop while we're still inside the host node.
let node : Fiber = root;
while (true) {
commitUnmount(node);
if (node.child) {
// TODO: Coroutines need to visit the stateNode.
node = node.child;
continue;
}
if (node === root) {
return;
}
while (!node.sibling) {
if (!node.return || node.return === root) {
return;
}
node = node.return;
}
node = node.sibling;
}
}

function commitDeletion(current : Fiber) : void {
// Recursively delete all host nodes from the parent.
// TODO: Error handling.
const parent = getHostParent(current);
if (!parent) {
return;
Expand All @@ -146,11 +174,17 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
let node : Fiber = current;
while (true) {
if (node.tag === HostComponent || node.tag === HostText) {
commitNestedUnmounts(node);
// After all the children have unmounted, it is now safe to remove the
// node from the tree.
removeChild(parent, node.stateNode);
} else if (node.child) {
// TODO: Coroutines need to visit the stateNode.
node = node.child;
continue;
} else {
commitUnmount(node);
if (node.child) {
// TODO: Coroutines need to visit the stateNode.
node = node.child;
continue;
}
}
if (node === current) {
return;
Expand All @@ -165,6 +199,17 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
}
}

function commitUnmount(current : Fiber) : void {
switch (current.tag) {
case ClassComponent: {
const instance = current.stateNode;
if (typeof instance.componentWillUnmount === 'function') {
instance.componentWillUnmount();
}
}
}
}

function commitWork(current : ?Fiber, finishedWork : Fiber) : void {
switch (finishedWork.tag) {
case ClassComponent: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,38 @@ describe('ReactIncrementalSideEffects', () => {

});

it('can deletes children either components, host or text', function() {

function Bar(props) {
return <span prop={props.children} />;
}

function Foo(props) {
return (
<div>
{props.show ? [
<div key="a" />,
<Bar key="b">Hello</Bar>,
'World',
] : []}
</div>
);
}

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

ReactNoop.render(<Foo show={false} />);
ReactNoop.flush();
expect(ReactNoop.root.children).toEqual([
div(),
]);

});

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

function Bar(props) {
Expand Down Expand Up @@ -459,4 +491,73 @@ describe('ReactIncrementalSideEffects', () => {
});

// TODO: Test that callbacks are not lost if an update is preempted.

it('calls componentWillUnmount after a deletion, even if nested', () => {

var ops = [];

class Bar extends React.Component {
componentWillUnmount() {
ops.push(this.props.name);
}
render() {
return <span />;
}
}

class Wrapper extends React.Component {
componentWillUnmount() {
ops.push('Wrapper');
}
render() {
return <Bar name={this.props.name} />;
}
}

function Foo(props) {
return (
<div>
{props.show ? [
<Bar key="a" name="A" />,
<Wrapper key="b" name="B" />,
<div key="cd">
<Bar name="C" />
<Wrapper name="D" />,
</div>,
[
<Bar key="e" name="E" />,
<Bar key="f" name="F" />,
],
] : []}
<div>
{props.show ? <Bar key="g" name="G" /> : null}
</div>
<Bar name="this should not unmount" />
</div>
);
}

ReactNoop.render(<Foo show={true} />);
ReactNoop.flush();
expect(ops).toEqual([]);

ReactNoop.render(<Foo show={false} />);
ReactNoop.flush();
expect(ops).toEqual([
'A',
'Wrapper',
'B',
'C',
'Wrapper',
'D',
'E',
'F',
'G',
]);

});

// TODO: Test that unmounts and deletions happen in the expected way for
// aborted and resumed render life-cycles.

});