From ceeb7df2eb1aa868c46a54d77cf79bab4c923aee Mon Sep 17 00:00:00 2001 From: Robert-Frampton Date: Mon, 23 Oct 2017 12:09:34 -0700 Subject: [PATCH] Rename willRender to willUpdate since it's related to shouldUpdate and move implementation to renderer --- packages/metal-component/src/Component.js | 10 --- packages/metal-component/test/Component.js | 27 ------- .../src/IncrementalDomRenderer.js | 23 +++++- .../test/IncrementalDomRenderer.js | 81 +++++++++++++++++++ packages/metal-jsx/test/JSXRenderer.js | 34 ++++++++ 5 files changed, 136 insertions(+), 39 deletions(-) diff --git a/packages/metal-component/src/Component.js b/packages/metal-component/src/Component.js index 24d82686..a32e5724 100644 --- 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 index 19382930..e2794ae9 100644 --- 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 index 9d1615d0..372defc0 100644 --- 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 index 9bd9b865..2911ef2a 100644 --- 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 index eed327f0..b69dc9e3 100644 --- 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() {