merged spike - 1 failing test now, WIP still

infernojs · Mar 23, 2016 · bf011f5 · bf011f5
2 parents 00707a4 + 12a1b71
commit bf011f5
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 23 deletions.
diff --git a/package.json b/package.json
@@ -28,7 +28,7 @@
     "isparta": "^4.0.0",
     "isparta-loader": "^2.0.0",
     "karma": "^0.13.22",
-    "karma-chrome-launcher": "^0.2.2",
+    "karma-chrome-launcher": "^0.2.3",
     "karma-coverage": "^0.5.5",
     "karma-coveralls": "^1.1.2",
     "karma-mocha": "^0.2.2",
@@ -42,7 +42,7 @@
     "npm-check-updates": "^2.6.1",
     "pre-commit": "^1.1.2",
     "rimraf": "^2.5.2",
-    "rollup": "^0.25.4",
+    "rollup": "^0.25.5",
     "rollup-plugin-babel": "2.4.0",
     "rollup-plugin-node-resolve": "^1.5.0",
     "rollup-plugin-replace": "^1.1.0",

diff --git a/packages/inferno-dom/dist/inferno-dom.es2015.js b/packages/inferno-dom/dist/inferno-dom.es2015.js
@@ -127,10 +127,6 @@ function isNumber(obj) {
 	return typeof obj === 'number';
 }
 
-function isPromise(obj) {
-	return obj instanceof Promise;
-}
-
 var delegatedEventsRegistry = {};
 
 // The issue with this, is that we can't stop the bubbling as we're traversing down the node tree, rather than up it
@@ -251,6 +247,7 @@ function replaceNode(lastNode, nextNode, parentDom, namespace, lifecycle, contex
 		nextNode.dom = _dom;
 		parentDom.replaceChild(_dom, parentDom.firstChild);
 	} else {
+		detachNode(lastNode);
 		var _dom2 = mountNode(nextNode, null, namespace, lifecycle, context, instance);
 		nextNode.dom = _dom2;
 		parentDom.replaceChild(_dom2, lastNode.dom);
@@ -266,7 +263,7 @@ function detachNode(node) {
 		instance.componentWillUnmount();
 		instance._unmounted = true;
 	}
-	var hooks = node.hooks;
+	var hooks = node.hooks || !isNullOrUndefined(instance) && instance.hooks;
 	if (!isNullOrUndefined(hooks)) {
 		if (!isNullOrUndefined(hooks.willDetach)) {
 			hooks.willDetach(node.dom);
@@ -970,12 +967,13 @@ function mountChildren(children, parentDom, namespace, lifecycle, context, insta
 	if (isArray(children)) {
 		for (var i = 0; i < children.length; i++) {
 			var child = children[i];
+			var childDefined = !isNullOrUndefined(child);
 
 			if (isStringOrNumber(child)) {
 				appendText(child, parentDom, false);
-			} else if (!isNullOrUndefined(child) && isArray(child)) {
+			} else if (childDefined && isArray(child)) {
 				mountChildren(child, parentDom, namespace, lifecycle, context, instance);
-			} else if (isPromise(child)) {
+			} else if (childDefined && !isNullOrUndefined(child.then)) {
 				(function () {
 					var placeholder = document.createTextNode('');
 
@@ -993,7 +991,7 @@ function mountChildren(children, parentDom, namespace, lifecycle, context, insta
 	} else {
 		if (isStringOrNumber(children)) {
 			appendText(children, parentDom, true);
-		} else if (isPromise(children)) {
+		} else if (!isNullOrUndefined(children) && !isNullOrUndefined(children.then)) {
 			(function () {
 				var placeholder = document.createTextNode('');
 

diff --git a/packages/inferno/dist/inferno-dom.js b/packages/inferno/dist/inferno-dom.js
@@ -133,10 +133,6 @@
 		return typeof obj === 'number';
 	}
 
-	function isPromise(obj) {
-		return obj instanceof Promise;
-	}
-
 	var delegatedEventsRegistry = {};
 
 	// The issue with this, is that we can't stop the bubbling as we're traversing down the node tree, rather than up it
@@ -257,6 +253,7 @@
 			nextNode.dom = _dom;
 			parentDom.replaceChild(_dom, parentDom.firstChild);
 		} else {
+			detachNode(lastNode);
 			var _dom2 = mountNode(nextNode, null, namespace, lifecycle, context, instance);
 			nextNode.dom = _dom2;
 			parentDom.replaceChild(_dom2, lastNode.dom);
@@ -272,7 +269,7 @@
 			instance.componentWillUnmount();
 			instance._unmounted = true;
 		}
-		var hooks = node.hooks;
+		var hooks = node.hooks || !isNullOrUndefined(instance) && instance.hooks;
 		if (!isNullOrUndefined(hooks)) {
 			if (!isNullOrUndefined(hooks.willDetach)) {
 				hooks.willDetach(node.dom);
@@ -976,12 +973,13 @@
 		if (isArray(children)) {
 			for (var i = 0; i < children.length; i++) {
 				var child = children[i];
+				var childDefined = !isNullOrUndefined(child);
 
 				if (isStringOrNumber(child)) {
 					appendText(child, parentDom, false);
-				} else if (!isNullOrUndefined(child) && isArray(child)) {
+				} else if (childDefined && isArray(child)) {
 					mountChildren(child, parentDom, namespace, lifecycle, context, instance);
-				} else if (isPromise(child)) {
+				} else if (childDefined && !isNullOrUndefined(child.then)) {
 					(function () {
 						var placeholder = document.createTextNode('');
 
@@ -999,7 +997,7 @@
 		} else {
 			if (isStringOrNumber(children)) {
 				appendText(children, parentDom, true);
-			} else if (isPromise(children)) {
+			} else if (!isNullOrUndefined(children) && !isNullOrUndefined(children.then)) {
 				(function () {
 					var placeholder = document.createTextNode('');
 

diff --git a/packages/inferno/dist/inferno-dom.min.js b/packages/inferno/dist/inferno-dom.min.js
diff --git a/src/DOM/__tests__/hooks.spec.browser.js b/src/DOM/__tests__/hooks.spec.browser.js
@@ -1,4 +1,5 @@
 import { render } from '../../DOM/rendering';
+import createElement from './../../core/createElement';
 
 
 describe('lifecycle hooks', () => {
@@ -695,4 +696,106 @@ describe('lifecycle hooks', () => {
 			expect(onComponentShouldUpdateDomNode).to.equal(expectedDomNode);
 		});
 	});
-});
+
+	describe('github issue with willDetach', () => {
+
+		it('should raise willDetach', () => {
+
+			let didAttach = false,
+				didCreate = false,
+				didDetach = false;
+
+			function addElement() {
+				var el = createElement(function(v0, v1, v2) {
+					return {
+						tag: 'h1',
+						attrs: {
+							class: 'something'
+						},
+						hooks: {
+							attached: function() {
+								didAttach = true;
+							},
+							created: function() {
+								didCreate = true;
+							},
+							willDetach: function() {
+								didDetach = true;
+							}
+						},
+						children: 'Hi there!'
+					};
+				});
+
+				render(
+					el,
+					container
+				);
+			}
+
+			function removeElement() {
+				render(
+					null,
+					container
+				);
+			}
+
+			addElement();
+			removeElement();
+
+			expect(didAttach).to.equal(true);
+			expect(didCreate).to.equal(true);
+			expect(didDetach).to.equal(true);
+
+
+		});
+
+		it('should raise willDetach', () => {
+
+			let didDetach = false;
+
+			function addElement() {
+				var el = createElement(function(v0, v1, v2) {
+					return {
+						tag: 'h1',
+						attrs: {
+							class: 'something'
+						},
+						hooks: {
+							willDetach: function() {
+								didDetach = true;
+							}
+						},
+						children: 'Hi there!'
+					};
+				});
+
+				render(
+					el,
+					container
+				);
+			}
+
+			function addElementTwo() {
+				var el = createElement(function(v0, v1, v2) {
+					return {
+						tag: 'h2',
+						attrs: {
+							class: 'something'
+						},
+						children: 'Another!'
+					};
+				});
+
+				render(
+					el,
+					container
+				);
+			}
+
+			addElement();
+			addElementTwo();
+			expect(didDetach).to.equal(true);
+		});
+	});
+});
diff --git a/src/DOM/mounting.js b/src/DOM/mounting.js
@@ -29,11 +29,13 @@ export function mountChildren(node, children, parentDom, namespace, lifecycle, c
 			const child = children[i];
 
 			if (isStringOrNumber(child)) {
+
 				isNonKeyed = true;
 				domChildren.push(appendText(child, parentDom, false));
 			} else if (!isNullOrUndefined(child) && isArray(child)) {
 				const virtualFragment = createVirtualFragment();
 
+
 				isNonKeyed = true;
 				mountChildren(node, child, virtualFragment, namespace, lifecycle, context, instance);
 				insertOrAppend(parentDom, virtualFragment);
@@ -58,8 +60,10 @@ export function mountChildren(node, children, parentDom, namespace, lifecycle, c
 	} else {
 		if (isStringOrNumber(children)) {
 			appendText(children, parentDom, true);
+
 		} else if (isPromise(children)) {
 			appendPromise(children, parentDom, null, namespace, lifecycle, context, instance);
+
 		} else {
 			mountNode(children, parentDom, namespace, lifecycle, context, instance);
 		}

diff --git a/src/DOM/utils.js b/src/DOM/utils.js
@@ -59,6 +59,7 @@ export function replaceNode(lastNode, nextNode, parentDom, namespace, lifecycle,
 		nextNode.dom = dom;
 		parentDom.replaceChild(dom, parentDom.firstChild);
 	} else {
+		detachNode(lastNode);
 		const dom = mountNode(nextNode, null, namespace, lifecycle, context, instance);
 		nextNode.dom = dom;
 		parentDom.replaceChild(dom, lastNode.dom);
@@ -74,7 +75,7 @@ export function detachNode(node) {
 		instance.componentWillUnmount();
 		instance._unmounted = true;
 	}
-	const hooks = node.hooks;
+	const hooks = node.hooks || !isNullOrUndefined(instance) && instance.hooks;
 	if (!isNullOrUndefined(hooks)) {
 		if (!isNullOrUndefined(hooks.willDetach)) {
 			hooks.willDetach(node.dom);

diff --git a/src/component/__tests__/components.spec.jsx.js b/src/component/__tests__/components.spec.jsx.js
@@ -1353,4 +1353,76 @@ describe('Components (JSX)', () => {
 			});
 		});
 	});
+
+	describe('updating child should not cause rendering parent to fail', () => {
+		it('should render parent correctly after child changes', () => {
+
+			let updateParent,
+				updateChild;
+
+			class Parent extends Component {
+				constructor(props) {
+					super(props);
+					this.state = {x: false};
+
+					updateParent = () => {
+						this.setState({x: true});
+					};
+				}
+
+				render() {
+					return (
+						<div>
+							<p>parent</p>
+							{!this.state.x? <ChildA /> :<ChildB />}
+						</div>
+					);
+				};
+			}
+
+			class ChildB extends Component {
+				constructor(props) {
+					super(props);
+				};
+				render() {
+					return (<div>Y</div>);
+				};
+			}
+
+			class ChildA extends Component {
+				constructor(props) {
+					super(props);
+					this.state = {z: false};
+
+					updateChild = () => {
+						this.setState({z: true});
+					}
+				};
+
+				render() {
+					if (!this.state.z)
+						return (<div>A</div>);
+
+					return (<SubChild />);
+				};
+			}
+
+			class SubChild extends Component {
+				constructor(props) {
+					super(props);
+				};
+
+				render() {
+					return (<div>B</div>);
+				};
+			}
+
+			render(<Parent />, container);
+			expect(container.innerHTML).to.equal('<div><p>parent</p><div>A</div></div>');
+			updateChild();
+			expect(container.innerHTML).to.equal('<div><p>parent</p><div>B</div></div>');
+			updateParent();
+			expect(container.innerHTML).to.equal('<div><p>parent</p><div>Y</div></div>');
+		});
+	})
 });
diff --git a/src/core/utils.js b/src/core/utils.js
@@ -76,7 +76,7 @@ export function isAttrAComponentHook(hook) {
 }
 
 export function isPromise(obj) {
-	return obj instanceof Promise;
+	return obj && obj.then;
 }
 
 export function replaceInArray(array, obj, newObj) {
@@ -85,4 +85,4 @@ export function replaceInArray(array, obj, newObj) {
 
 export function removeInArray(array, obj) {
 	array.splice(array.indexOf(obj), 1);
-}
+}