Rename willRender to willUpdate since it's related to shouldUpdate an…

…d move implementation to renderer
metal · Oct 23, 2017 · ceeb7df · ceeb7df
1 parent ef3fbc4
commit ceeb7df
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 39 deletions.
diff --git a/packages/metal-component/src/Component.js b/packages/metal-component/src/Component.js
@@ -527,9 +527,6 @@ class Component extends EventEmitter {
 	 *     be called manually later to actually attach it to the dom.
 	 */
 	renderComponent(opt_parentElement) {
-		const firstRender = !this.hasRendererRendered_;
-		this.emit('willRender', firstRender);
-		this.willRender(firstRender);
 		if (!this.hasRendererRendered_) {
 			if (!this.serverSide_ && window.__METAL_DEV_TOOLS_HOOK__) {
 				window.__METAL_DEV_TOOLS_HOOK__(this);
@@ -689,13 +686,6 @@ class Component extends EventEmitter {
 	 * Lifecycle. Fires before component is detached from the DOM.
 	 */
 	willDetach() {}
-
-	/**
-	 * Lifecycle. Fires whenever the component is about to render.
-	 * @param {boolean} firstRender Flag indicating if this will be the
-	 *     component's first render.
-	 */
-	willRender() {}
 }
 
 /**

diff --git a/packages/metal-component/test/Component.js b/packages/metal-component/test/Component.js
@@ -136,33 +136,6 @@ describe('Component', function() {
 			assert.strictEqual('.sibling', attachData.sibling);
 		});
 
-		it('should run "willRender" lifecycle method when the component about to render', function() {
-			class TestComponent extends Component {
-			}
-			sinon.spy(TestComponent.prototype, 'willRender');
-			sinon.spy(TestComponent.prototype, 'rendered');
-			comp = new TestComponent();
-
-			assert.ok(comp.willRender.calledBefore(comp.rendered));
-			assert.strictEqual(1, comp.willRender.callCount);
-			assert.ok(comp.willRender.args[0][0]);
-		});
-
-		it('should emit "willRender" event when the component about to render', function() {
-			var listener = sinon.stub();
-			class TestComponent extends Component {
-				created() {
-					this.on('willRender', listener);
-				}
-			}
-			sinon.spy(TestComponent.prototype, 'rendered');
-			comp = new TestComponent();
-
-			assert.ok(listener.calledBefore(comp.rendered));
-			assert.strictEqual(1, listener.callCount);
-			assert.ok(listener.args[0][0]);
-		});
-
 		it('should run "rendered" lifecycle method when the component is rendered', function() {
 			class TestComponent extends Component {
 			}

diff --git a/packages/metal-incremental-dom/src/IncrementalDomRenderer.js b/packages/metal-incremental-dom/src/IncrementalDomRenderer.js
@@ -154,10 +154,14 @@ class IncrementalDomRenderer extends ComponentRenderer.constructor {
 	 * @param {!Component} component
 	 */
 	renderInsidePatch(component) {
+		const changes = getChanges(component);
+
 		const shouldRender = !component.wasRendered ||
-			this.shouldUpdate(component, getChanges(component)) ||
+			this.shouldUpdate(component, changes) ||
 			IncrementalDOM.currentPointer() !== component.element;
 		if (shouldRender) {
+			this.willUpdate_(component, changes);
+
 			render(component);
 		} else if (component.element) {
 			this.skipRender();
@@ -218,10 +222,25 @@ class IncrementalDomRenderer extends ComponentRenderer.constructor {
 	 * @param {!Component} component
 	 */
 	update(component) {
-		if (this.shouldUpdate(component, getChanges(component))) {
+		const changes = getChanges(component);
+		if (this.shouldUpdate(component, changes)) {
+			this.willUpdate_(component, changes);
+
 			this.patch(component);
 		}
 	}
+
+	/**
+	 * Invokes component's "willUpdate" lifecycle method if applicable.
+	 * @param {!Component} component
+	 * @param {Object} changes
+	 */
+	willUpdate_(component, changes) {
+		if (!component.willUpdate || !component.wasRendered || !changes) {
+			return;
+		}
+		component.willUpdate(...this.buildShouldUpdateArgs(changes));
+	}
 }
 
 const renderer = new IncrementalDomRenderer();

diff --git a/packages/metal-incremental-dom/test/IncrementalDomRenderer.js b/packages/metal-incremental-dom/test/IncrementalDomRenderer.js
@@ -2768,6 +2768,87 @@ describe('IncrementalDomRenderer', function() {
 		});
 	});
 
+	describe('Function - willUpdate', function() {
+		it('should run "willUpdate" lifecycle method when the component is about to render', function(done) {
+			class TestComponent extends Component {
+				render() {}
+			}
+			TestComponent.prototype.willUpdate = sinon.stub();
+			TestComponent.RENDERER = IncrementalDomRenderer;
+			TestComponent.STATE = {
+				foo: {
+					value: 'foo'
+				}
+			};
+
+			component = new TestComponent();
+
+			async.nextTick(function() {
+				component.foo = 'foo2';
+
+				async.nextTick(function() {
+					sinon.assert.callCount(component.willUpdate, 1);
+					sinon.assert.calledWith(component.willUpdate, {
+						foo: {
+							key: 'foo',
+							newVal: 'foo2',
+							prevVal: 'foo'
+						}
+					});
+
+					done();
+				});
+			});
+		});
+
+		it('should run "willUpdate" lifecycle method of nested component when it is about to render', function(done) {
+			const listener = sinon.stub();
+			class TestChildComponent extends Component {
+				render() {}
+			}
+			TestChildComponent.prototype.willUpdate = sinon.stub();
+			TestChildComponent.RENDERER = IncrementalDomRenderer;
+			TestChildComponent.STATE = {
+				foo: {
+					value: 'foo'
+				}
+			};
+
+			class TestComponent extends Component {
+				render() {
+					IncDom.elementOpen('div');
+					IncDom.elementVoid(TestChildComponent, null, null, 'ref', 'child', 'foo', this.foo);
+					IncDom.elementClose('div');
+				}
+			}
+			TestComponent.RENDERER = IncrementalDomRenderer;
+			TestComponent.STATE = {
+				foo: {
+					value: 'foo'
+				}
+			};
+
+			component = new TestComponent();
+
+			const child = component.components.child;
+
+			async.nextTick(function() {
+				component.foo = 'foo2';
+
+				async.nextTick(function() {
+					sinon.assert.callCount(child.willUpdate, 1);
+					assert.deepEqual(child.willUpdate.args[0][0].foo, {
+						key: 'foo',
+						newVal: 'foo2',
+						prevVal: 'foo'
+					});
+
+					done();
+				});
+			});
+		});
+	});
+
 	describe('Componentless function tags', function() {
 		it('should render componentless function passed as incremental dom tag', function() {
 			var TestFunction = ({foo}) => {

diff --git a/packages/metal-jsx/test/JSXRenderer.js b/packages/metal-jsx/test/JSXRenderer.js
@@ -248,6 +248,40 @@ describe('JSXRenderer', function() {
 		});
 	});
 
+	it('should pass both state and prop changes to willUpdate', function(done) {
+		class TestComponent extends TestJSXComponent {
+			willUpdate() {}
+		}
+		TestComponent.PROPS = {
+			bar: {
+			}
+		};
+		TestComponent.STATE = {
+			foo: {
+			}
+		};
+
+		component = new TestComponent();
+		sinon.stub(component, 'willUpdate');
+		component.props.bar = 'bar';
+		component.state.foo = 'foo';
+		component.once('stateChanged', function() {
+			assert.strictEqual(1, component.willUpdate.callCount);
+
+			const stateChanges = component.willUpdate.args[0][0];
+			assert.ok(stateChanges.foo);
+			assert.strictEqual('foo', stateChanges.foo.newVal);
+			assert.strictEqual(undefined, stateChanges.foo.prevVal);
+
+			const propChanges = component.willUpdate.args[0][1];
+			assert.ok(propChanges.bar);
+			assert.strictEqual('bar', propChanges.bar.newVal);
+			assert.strictEqual(undefined, propChanges.bar.prevVal);
+
+			done();
+		});
+	});
+
 	it('should reuse elements correctly when child skips update', function(done) {
 		class ChildComponent extends TestJSXComponent {
 			render() {