Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add willReceiveState and willReceiveProps lifecycle methods to Component and JSXComponent respectively #270

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions packages/metal-component/src/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,21 @@ import { EventEmitter, EventHandler } from 'metal-events';
* created() {
* }
*
* willRender() {
* }
*
* rendered() {
* }
*
* willAttach() {
* }
*
* attached() {
* }
*
* willDetach() {
* }
*
* detached() {
* }
*
Expand Down Expand Up @@ -125,6 +134,7 @@ class Component extends EventEmitter {
this.setUpDataManager_();
this.setUpSyncUpdates_();

this.on('stateWillChange', this.handleStateWillChange_);
this.on('stateChanged', this.handleComponentStateChanged_);
this.on('eventsChanged', this.onEventsChanged_);
this.addListenersFromObj_(this.dataManager_.get(this, 'events'));
Expand Down Expand Up @@ -172,6 +182,8 @@ class Component extends EventEmitter {
*/
attach(opt_parentElement, opt_siblingElement) {
if (!this.inDocument) {
this.emit('willAttach');
this.willAttach();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Robert-Frampton, based on componentWillMount():

componentWillMount() is invoked immediately before mounting occurs. It is called before render(), therefore setting state synchronously in this method will not trigger a re-rendering. Avoid introducing any side-effects or subscriptions in this method.

Can we assume the same with this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jbalsas

So an area where Metal is already different than React is that our render actually happens before attach. This is how the lifecycle is ordered on first render.

created > willRender > rendered > willAttach > attached > willDetach > detached > disposed

I think the only way we can mirror React completely is to do a breaking change and change the order so that attachment happens before render, but obviously we can't do that for 2.14. Although we could move willAttach only, but it feels weird to me.

created > willAttach > willRender > rendered > attached > willDetach > detached > disposed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine for now... I think we are entitled to some little quirks of our own ;)

I think we just need to improve our Component Lifecycle documentation so everything is clear :)

this.attachElement(opt_parentElement, opt_siblingElement);
this.inDocument = true;
this.attachData_ = {
Expand Down Expand Up @@ -238,6 +250,8 @@ class Component extends EventEmitter {
*/
detach() {
if (this.inDocument) {
this.emit('willDetach');
this.willDetach();
if (this.element && this.element.parentNode) {
this.element.parentNode.removeChild(this.element);
}
Expand Down Expand Up @@ -377,6 +391,18 @@ class Component extends EventEmitter {
});
}

/**
* Fires before state batch changes. Provides hook point for modifying
* state.
* @param {Event} event
* @protected
*/
handleStateWillChange_(event) {
if (this.willReceiveState) {
this.willReceiveState(event.changes);
}
}

/**
* Checks if this component has sync updates enabled.
* @return {boolean}
Expand Down Expand Up @@ -501,6 +527,9 @@ 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on componentWillUpdate(), the signature for this method should be:

componentWillUpdate(nextProps, nextState)

Furthermore:

componentWillUpdate() is invoked immediately before rendering when new props or state are being received. Use this as an opportunity to perform preparation before an update occurs. This method is not called for the initial render.

Finally, do we have the last note in mind?

componentWillUpdate() will not be invoked if shouldComponentUpdate() returns false.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I update this in the new pull. I changed the name to willUpdate since it's closely related to shouldUpdate and receives the same arguments.

if (!this.hasRendererRendered_) {
if (!this.serverSide_ && window.__METAL_DEV_TOOLS_HOOK__) {
window.__METAL_DEV_TOOLS_HOOK__(this);
Expand Down Expand Up @@ -650,6 +679,23 @@ class Component extends EventEmitter {
validatorEventsFn_(val) {
return !isDefAndNotNull(val) || isObject(val);
}

/**
* Lifecycle. Fires before the component has been attached to the DOM.
*/
willAttach() {}

/**
* 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() {}
}

/**
Expand Down
158 changes: 158 additions & 0 deletions packages/metal-component/test/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,29 @@ describe('Component', function() {
assert.ok(comp.inDocument);
});

it('should run "willAttach" lifecycle method when the component about to attach', function() {
class TestComponent extends Component {
}
sinon.spy(TestComponent.prototype, 'willAttach');
comp = new TestComponent();

assert.ok(comp.willAttach.calledBefore(Component.prototype.attached));
assert.strictEqual(1, comp.willAttach.callCount);
});

it('should emit "willAttach" lifecycle method when the component about to attach', function() {
var listener = sinon.stub();
class TestComponent extends Component {
created() {
this.on('willAttach', listener);
}
}
comp = new TestComponent();

assert.ok(listener.calledBefore(Component.prototype.attached));
assert.strictEqual(1, listener.callCount);
});

it('should emit "attached" event when component is attached', function() {
comp = new Component({}, false);
var listener = sinon.stub();
Expand All @@ -113,6 +136,33 @@ 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 {
}
Expand Down Expand Up @@ -142,6 +192,37 @@ describe('Component', function() {
assert.strictEqual(comp, comp.attach());
});

it('should run "willDetach" lifecycle method when the component about to detach', function() {
class TestComponent extends Component {
}
sinon.spy(TestComponent.prototype, 'willDetach');
comp = new TestComponent();

assert.strictEqual(0, comp.willDetach.callCount);

comp.detach();

assert.ok(comp.willDetach.calledBefore(Component.prototype.detached));
assert.strictEqual(1, comp.willDetach.callCount);
});

it('should emit "willDetach" lifecycle method when the component about to detach', function() {
var listener = sinon.stub();
class TestComponent extends Component {
created() {
this.on('willDetach', listener);
}
}
comp = new TestComponent();

assert.strictEqual(0, listener.callCount);

comp.detach();

assert.ok(listener.calledBefore(Component.prototype.detached));
assert.strictEqual(1, listener.callCount);
});

it('should dispose component', function() {
comp = new Component();

Expand Down Expand Up @@ -480,6 +561,83 @@ describe('Component', function() {
new CustomComponent();
});
});

it('should allow changes to state in "willReceiveState" without triggering multiple renders', function(done) {
const renderStub = sinon.stub();
let count = 0;

class CustomRenderer extends ComponentRenderer.constructor {
update(component) {
renderStub();

component.element.innerHTML = `${component.bar}:${component.foo}`;
component.informRendered();
}
}

class TestComponent extends Component {
willReceiveState(changes) {
this.foo = 'foo' + count;

count++;
}
}
TestComponent.STATE = {
bar: {
value: 'bar'
},

foo: {
value: 'foo'
}
};
TestComponent.RENDERER = new CustomRenderer();

comp = new TestComponent();

comp.bar = 'bar2';
comp.once('rendered', function() {
assert.equal(comp.element.innerHTML, 'bar2:foo0');

async.nextTick(function() {
comp.bar = 'bar3';
comp.once('rendered', function() {
assert.equal(renderStub.callCount, 2);
assert.equal(comp.element.innerHTML, 'bar3:foo1');

done();
});
});
});
});

it('should pass changed state data to "willReceiveState" method', function(done) {
class TestComponent extends Component {
}
TestComponent.prototype.willReceiveState = sinon.stub();
TestComponent.STATE = {
foo: {
value: 'foo'
}
};

comp = new TestComponent();

comp.foo = 'foo2';

async.nextTick(function() {
assert.equal(comp.willReceiveState.callCount, 1);
assert.deepEqual(comp.willReceiveState.args[0][0], {
foo: {
key: 'foo',
newVal: 'foo2',
prevVal: 'foo'
}
});

done();
});
});
});

describe('Render', function() {
Expand Down
12 changes: 12 additions & 0 deletions packages/metal-jsx/src/JSXComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ class JSXComponent extends Component {
return IncrementalDomRenderer.render(...args);
}

/**
* Fires before state batch changes. Provides hook point for modifying
* state.
* @param {Event} event
* @protected
*/
handleStateWillChange_(event) {
if (event.type !== 'state' && this.willReceiveProps) {
this.willReceiveProps(event.changes);
}
}

/**
* Returns props that are not used or declared in the component.
* @return {Object} Object containing props
Expand Down
76 changes: 76 additions & 0 deletions packages/metal-jsx/test/JSXComponent.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

import { async } from 'metal';
import dom from 'metal-dom';
import JSXComponent from '../src/JSXComponent';

Expand Down Expand Up @@ -307,6 +308,81 @@ describe('JSXComponent', function() {
});
});

it('should allow changes to state in "willReceiveProps" without triggering multiple renders', function(done) {
const renderStub = sinon.stub();
let count = 0;

class TestComponent extends JSXComponent {
render() {
renderStub();

return <div class="component">{this.props.bar}:{this.state.foo}</div>;
}

willReceiveProps(data) {
this.state.foo = 'foo' + count;

count++;
}
}
TestComponent.STATE = {
foo: {
value: 'foo'
}
};
TestComponent.PROPS = {
bar: {
value: 'bar'
}
};

component = new TestComponent();

component.props.bar = 'bar2';

component.once('rendered', function() {
assert.equal(component.element.innerHTML, 'bar2:foo0');

async.nextTick(function() {
component.props.bar = 'bar3';
component.once('rendered', function() {
assert.equal(component.element.innerHTML, 'bar3:foo1');
assert.equal(renderStub.callCount, 3);

done();
});
});
});
});

it('should pass changed props data to "willReceiveProps" method', function(done) {
class TestComponent extends JSXComponent {
}
TestComponent.prototype.willReceiveProps = sinon.stub();
TestComponent.PROPS = {
foo: {
value: 'foo'
}
};

component = new TestComponent();

component.props.foo = 'foo2';

async.nextTick(function() {
assert.equal(component.willReceiveProps.callCount, 1);
assert.deepEqual(component.willReceiveProps.args[0][0], {
foo: {
key: 'foo',
newVal: 'foo2',
prevVal: 'foo'
}
});

done();
});
});

it('component.element and child.element should be the same', function() {
class ChildComponent extends JSXComponent {
render() {
Expand Down
Loading