Skip to content

Commit

Permalink
Fix return value of ReactDOMFiber.unmountComponentAtNode (#9619)
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiebits committed May 6, 2017
1 parent 767a5d6 commit 542dac4
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
2 changes: 2 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,8 @@ src/renderers/dom/shared/__tests__/ReactEventListener-test.js

src/renderers/dom/shared/__tests__/ReactMount-test.js
* throws when given a non-node
* returns false on non-React containers
* returns true on React containers
* throws when given a string
* throws when given a factory
* should render different components in same root
Expand Down
9 changes: 7 additions & 2 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,16 @@ var ReactDOM = {
}

// Unmount should not be batched.
return DOMRenderer.unbatchedUpdates(() => {
return renderSubtreeIntoContainer(null, null, container, () => {
DOMRenderer.unbatchedUpdates(() => {
renderSubtreeIntoContainer(null, null, container, () => {
container._reactRootContainer = null;
});
});
// If you call unmountComponentAtNode twice in quick succession, you'll
// get `true` twice. That's probably fine?
return true;
} else {
return false;
}
},

Expand Down
15 changes: 15 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactMount-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ describe('ReactMount', () => {
'unmountComponentAtNode(...): Target container is not a DOM element.',
);
});

it('returns false on non-React containers', () => {
var d = document.createElement('div');
d.innerHTML = '<b>hellooo</b>';
expect(ReactDOM.unmountComponentAtNode(d)).toBe(false);
expect(d.textContent).toBe('hellooo');
});

it('returns true on React containers', () => {
var d = document.createElement('div');
ReactDOM.render(<b>hellooo</b>, d);
expect(d.textContent).toBe('hellooo');
expect(ReactDOM.unmountComponentAtNode(d)).toBe(true);
expect(d.textContent).toBe('');
});
});

it('throws when given a string', () => {
Expand Down

0 comments on commit 542dac4

Please sign in to comment.