Merge pull request #1781 from preactjs/portals_destroyer

Fix Portal children always being mounted

compat/src/index.js

@@ -103,12 +103,17 @@ function Portal(props) {
 			_this._container = container;
 			// Render our wrapping element into temp.
 			preactRender(wrap, container, _this._temp);
+			_this._children = this._temp._children;
 		}
 		else {
 			// When we have mounted and the vnode is present it means the
 			// props have changed or a parent is triggering a rerender.
-			// This implies we only need to call render.
-			render(wrap, container);
+			// This implies we only need to call render. But we need to keep
+			// the old tree around, otherwise will treat the vnodes as new and
+			// will wrongly call `componentDidMount` on them
+			container._children = _this._children;
+			preactRender(wrap, container);
+			_this._children = container._children;
 		}
 	}
 	// When we come from a conditional render, on a mounted

compat/test/browser/portals.test.js

@@ -384,4 +384,48 @@ describe('Portal', () => {
 		rerender();
 		expect(scratch.innerHTML).to.equal('<div><p>Hello</p></div>');
 	});
+
+	it('should not unmount when parent renders', () => {
+		let root = document.createElement('div');
+		let dialog = document.createElement('div');
+		dialog.id = 'container';
+
+		scratch.appendChild(root);
+		scratch.appendChild(dialog);
+
+		let spy = sinon.spy();
+		class Child extends Component {
+			componentDidMount() {
+				spy();
+			}
+
+			render() {
+				return <div id="child">child</div>;
+			}
+		}
+
+		let spyParent = sinon.spy();
+		class App extends Component {
+			componentDidMount() {
+				spyParent();
+			}
+			render() {
+				return <div>{createPortal(<Child />, dialog)}</div>;
+			}
+		}
+
+		render(<App />, root);
+		let dom = document.getElementById('child');
+		expect(spyParent).to.be.calledOnce;
+		expect(spy).to.be.calledOnce;
+
+		// Render twice to trigger update scenario
+		render(<App />, root);
+		render(<App />, root);
+
+		let domNew = document.getElementById('child');
+		expect(dom).to.equal(domNew);
+		expect(spyParent).to.be.calledOnce;
+		expect(spy).to.be.calledOnce;
+	});
 });