Merge 69cabb8 into b03bb36

preactjs · Jun 20, 2017 · 7f91d97 · 7f91d97
2 parents b03bb36 + 69cabb8
commit 7f91d97
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 17 deletions.
diff --git a/src/vdom/component.js b/src/vdom/component.js
@@ -144,12 +144,18 @@ export function renderComponent(component, opts, mountAll, isChild) {
 		if (initialBase && base!==initialBase && inst!==initialChildComponent) {
 			let baseParent = initialBase.parentNode;
 			if (baseParent && base!==baseParent) {
-				baseParent.replaceChild(base, initialBase);
 
 				if (!toUnmount) {
+					baseParent.replaceChild(base, initialBase);
 					initialBase._component = null;
 					recollectNodeTree(initialBase, false);
 				}
+				// @TODO Formerly the initialBase was always replaced by the base
+				//		 but somehow all tests passes also if its only called at not unmounting.
+				//       These else condition was the original fix.
+				// else {
+				// 	baseParent.insertBefore(base, initialBase);
+				// }
 			}
 		}
 
@@ -264,10 +270,11 @@ export function unmountComponent(component) {
 
 		component.nextBase = base;
 
-		removeNode(base);
 		collectComponent(component);
-
 		removeChildren(base);
+
+		// Remove Node as last, so all childs exists when componentWillUnmount is called
+		removeNode(base);
 	}
 
 	if (component.__ref) component.__ref(null);

diff --git a/src/vdom/diff.js b/src/vdom/diff.js
@@ -262,11 +262,12 @@ export function recollectNodeTree(node, unmountOnly) {
 		// (this is part of the React spec, and smart for unsetting references)
 		if (node[ATTR_KEY]!=null && node[ATTR_KEY].ref) node[ATTR_KEY].ref(null);
 
+		removeChildren(node);
+
+		// Remove Node as last, so all childs exists when componentWillUnmount is called
 		if (unmountOnly===false || node[ATTR_KEY]==null) {
 			removeNode(node);
 		}
-
-		removeChildren(node);
 	}
 }
 

diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js
@@ -456,42 +456,74 @@ describe('Lifecycle methods', () => {
 					expect(document.getElementById('InnerDiv'), 'Inner componentDidMount').to.exist;
 				}
 				componentWillUnmount() {
-					// @TODO Component mounted into elements (non-components)
-					// are currently unmounted after those elements, so their
-					// DOM is unmounted prior to the method being called.
-					//expect(document.getElementById('InnerDiv'), 'Inner componentWillUnmount').to.exist;
+					expect(document.getElementById('InnerDiv'), 'Inner componentWillUnmount').to.exist;
 					setTimeout( () => {
 						expect(document.getElementById('InnerDiv'), 'Inner after componentWillUnmount').to.not.exist;
 					}, 0);
 				}
 
 				render() {
-					return <div id="InnerDiv" />;
+					return <div id="InnerDiv">
+						<Innerest />
+					</div>;
+				}
+			}
+
+			class Innerest extends  Component {
+				componentWillMount() {
+					expect(document.getElementById('InnerestDiv'), 'Innerest componentWillMount').to.not.exist;
+				}
+				componentDidMount() {
+					expect(document.getElementById('InnerestDiv'), 'Innerest componentDidMount').to.exist;
+				}
+				componentWillUnmount() {
+					expect(document.getElementById('InnerestDiv'), 'Innerest componentWillUnmount').to.exist;
+					setTimeout( () => {
+						expect(document.getElementById('InnerestDiv'), 'Innerest after componentWillUnmount').to.not.exist;
+					}, 0);
+				}
+
+				render() {
+					return <div id="InnerestDiv"/>;
 				}
 			}
 
 			let proto = Inner.prototype;
+			let proto_innerest = Innerest.prototype;
 			let spies = ['componentWillMount', 'componentDidMount', 'componentWillUnmount'];
 			spies.forEach( s => sinon.spy(proto, s) );
+			spies.forEach( s => sinon.spy(proto_innerest, s) );
 
 			let reset = () => spies.forEach( s => proto[s].reset() );
+			let reset_innerest = () => spies.forEach( s => proto_innerest[s].reset() );
 
 			render(<Outer />, scratch);
-			expect(proto.componentWillMount).to.have.been.called;
-			expect(proto.componentWillMount).to.have.been.calledBefore(proto.componentDidMount);
-			expect(proto.componentDidMount).to.have.been.called;
+			expect(proto.componentWillMount, 'Inner componentWillMount').to.have.been.called;
+			expect(proto.componentWillMount, 'Inner componentWillMount').to.have.been.calledBefore(proto.componentDidMount);
+			expect(proto.componentDidMount, 'Inner componentDidMount').to.have.been.called;
+
+			expect(proto_innerest.componentWillMount, 'Innerest componentWillMount').to.have.been.called;
+			expect(proto_innerest.componentWillMount, 'Innerest componentWillMount').to.have.been.calledBefore(proto_innerest.componentDidMount);
+			expect(proto_innerest.componentDidMount, 'Innerest componentDidMount').to.have.been.called;
 
 			reset();
+			reset_innerest();
 			setState({ show:false });
 
-			expect(proto.componentWillUnmount).to.have.been.called;
+			expect(proto.componentWillUnmount, 'Inner componentWillUnmount').to.have.been.called;
+			expect(proto_innerest.componentWillUnmount, 'Innerest componentWillUnmount').to.have.been.called;
 
 			reset();
+			reset_innerest();
 			setState({ show:true });
 
-			expect(proto.componentWillMount).to.have.been.called;
-			expect(proto.componentWillMount).to.have.been.calledBefore(proto.componentDidMount);
-			expect(proto.componentDidMount).to.have.been.called;
+			expect(proto.componentWillMount, 'Inner componentWillMount').to.have.been.called;
+			expect(proto.componentWillMount, 'Inner componentWillMount').to.have.been.calledBefore(proto.componentDidMount);
+			expect(proto.componentDidMount, 'Inner componentDidMount').to.have.been.called;
+
+			expect(proto_innerest.componentWillMount, 'Innerest componentWillMount').to.have.been.called;
+			expect(proto_innerest.componentWillMount, 'Innerest componentWillMount').to.have.been.calledBefore(proto_innerest.componentDidMount);
+			expect(proto_innerest.componentDidMount, 'Innerest componentDidMount').to.have.been.called;
 		});
 
 		it('should remove this.base for HOC', () => {