Stops trying to reuse disposed components - Fixes #155

metal · Sep 14, 2016 · 71a7e66 · 71a7e66
1 parent 380cccd
commit 71a7e66
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 3 deletions.
diff --git a/packages/metal-incremental-dom/src/IncrementalDomRenderer.js b/packages/metal-incremental-dom/src/IncrementalDomRenderer.js
@@ -539,12 +539,16 @@ class IncrementalDomRenderer extends ComponentRenderer {
 	}
 
 	/**
-	 * Checks if this renderer's component is the owner of the given component.
+	 * Checks if the given component can be a match for a constructor.
 	 * @param {!Component} comp
+	 * @param {!function()} Ctor
 	 * @return {boolean}
 	 * @protected
 	 */
-	isOwner_(comp) {
+	isMatch_(comp, Ctor) {
+		if (!comp || comp.constructor !== Ctor || comp.isDisposed()) {
+			return false;
+		}
 		return comp.getRenderer().getOwner() === this.component_;
 	}
 
@@ -559,7 +563,7 @@ class IncrementalDomRenderer extends ComponentRenderer {
 	 * @protected
 	 */
 	match_(comp, Ctor, config) {
-		if (!comp || comp.constructor !== Ctor || !this.isOwner_(comp)) {
+		if (!this.isMatch_(comp, Ctor)) {
 			comp = new Ctor(config, false);
 		}
 		if (comp.wasRendered) {

diff --git a/packages/metal-incremental-dom/test/IncrementalDomRenderer.js b/packages/metal-incremental-dom/test/IncrementalDomRenderer.js
@@ -1324,6 +1324,43 @@ describe('IncrementalDomRenderer', function() {
 			});
 		});
 
+		it('should not throw error trying to reuse component that was disposed', function(done) {
+			const childInstances = [];
+			class TestChildComponent extends Component {
+				created() {
+					childInstances.push(this);
+				}
+				render() {
+					IncDom.elementVoid('span');
+				}
+			}
+			TestChildComponent.RENDERER = IncrementalDomRenderer;
+
+			class TestComponent extends Component {
+				render() {
+					IncDom.elementVoid(TestChildComponent);
+				}
+			}
+			TestComponent.RENDERER = IncrementalDomRenderer;
+			TestComponent.STATE = {
+				foo: {
+				}
+			};
+
+			component = new TestComponent();
+			assert.strictEqual(1, childInstances.length);
+			childInstances[0].dispose();
+
+			component.foo = true;
+			component.once('stateSynced', function() {
+				assert.strictEqual(2, childInstances.length);
+				assert.ok(childInstances[0].isDisposed());
+				assert.ok(!childInstances[1].isDisposed());
+				assert.strictEqual(component.element, childInstances[1].element);
+				done();
+			});
+		});
+
 		it('should reuse previous internal state data on sub component rerender', function(done) {
 			ChildComponent.STATE = {
 				foo: {