Skip to content

Commit

Permalink
[BUGFIX release] Ensure init is completed before didReceiveAttrs
Browse files Browse the repository at this point in the history
…fires.

Given the following example:

```javascript
export default Ember.Component.extend({
  init() {
    this._super(...arguments);

    this.wasInitWithSomeThing = !!this.attrs.someThing;
  },

  didReceiveAttrs() {
    this._super(...arguments);

    if (this.wasInitWithSomeThing) {
      // do custom logic based on being `init` with `someThing`
    }
  }
});
```

Currently, the `didReceiveAttrs` (and also `didInitAttrs`) is firing
from within the `this._super()` call in `init` (because we trigger these
hooks in `Ember.View`'s `init`). Which means that `didReceiveAttrs` can
not have access to things set in `init`.

This change allows us to avoid that circumstance, and ensure that `init`
is fully completed before `didReceiveAttrs` is fired and also ensures
taht `didReceiveAttrs`/`didInitAttrs` is still fired from within the
`constructor` before observation is started.
  • Loading branch information
rwjblue committed Sep 1, 2015
1 parent 4fbc27d commit 7315671
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ if (isEnabled('ember-htmlbars-component-generation')) {
}

styles.forEach(style => {
function invoke(name, hash) {
function invoke(name, hash = {}) {
if (style.name === 'curly') {
let attrs = Object.keys(hash).map(k => `${k}=${val(hash[k])}`).join(' ');
return `{{${name} ${attrs}}}`;
Expand Down Expand Up @@ -87,6 +87,7 @@ styles.forEach(style => {
this.label = label;
components[label] = this;
this._super.apply(this, arguments);
pushHook(label, 'init');
},

didInitAttrs(options) {
Expand Down Expand Up @@ -147,9 +148,9 @@ styles.forEach(style => {
let bottomAttrs = { website: 'tomdale.net' };

deepEqual(hooks, [
hook('top', 'didInitAttrs', { attrs: topAttrs }), hook('top', 'didReceiveAttrs', { newAttrs: topAttrs }), hook('top', 'willRender'),
hook('middle', 'didInitAttrs', { attrs: middleAttrs }), hook('middle', 'didReceiveAttrs', { newAttrs: middleAttrs }), hook('middle', 'willRender'),
hook('bottom', 'didInitAttrs', { attrs: bottomAttrs }), hook('bottom', 'didReceiveAttrs', { newAttrs: bottomAttrs }), hook('bottom', 'willRender'),
hook('top', 'init'), hook('top', 'didInitAttrs', { attrs: topAttrs }), hook('top', 'didReceiveAttrs', { newAttrs: topAttrs }), hook('top', 'willRender'),
hook('middle', 'init'), hook('middle', 'didInitAttrs', { attrs: middleAttrs }), hook('middle', 'didReceiveAttrs', { newAttrs: middleAttrs }), hook('middle', 'willRender'),
hook('bottom', 'init'), hook('bottom', 'didInitAttrs', { attrs: bottomAttrs }), hook('bottom', 'didReceiveAttrs', { newAttrs: bottomAttrs }), hook('bottom', 'willRender'),
hook('bottom', 'didInsertElement'), hook('bottom', 'didRender'),
hook('middle', 'didInsertElement'), hook('middle', 'didRender'),
hook('top', 'didInsertElement'), hook('top', 'didRender')
Expand Down Expand Up @@ -238,6 +239,7 @@ styles.forEach(style => {
this.label = label;
components[label] = this;
this._super.apply(this, arguments);
pushHook(label, 'init');
},

didInitAttrs(options) {
Expand Down Expand Up @@ -298,9 +300,9 @@ styles.forEach(style => {
let bottomAttrs = { twitterMiddle: '@tomdale' };

deepEqual(hooks, [
hook('top', 'didInitAttrs', { attrs: topAttrs }), hook('top', 'didReceiveAttrs', { newAttrs: topAttrs }), hook('top', 'willRender'),
hook('middle', 'didInitAttrs', { attrs: middleAttrs }), hook('middle', 'didReceiveAttrs', { newAttrs: middleAttrs }), hook('middle', 'willRender'),
hook('bottom', 'didInitAttrs', { attrs: bottomAttrs }), hook('bottom', 'didReceiveAttrs', { newAttrs: bottomAttrs }), hook('bottom', 'willRender'),
hook('top', 'init'), hook('top', 'didInitAttrs', { attrs: topAttrs }), hook('top', 'didReceiveAttrs', { newAttrs: topAttrs }), hook('top', 'willRender'),
hook('middle', 'init'), hook('middle', 'didInitAttrs', { attrs: middleAttrs }), hook('middle', 'didReceiveAttrs', { newAttrs: middleAttrs }), hook('middle', 'willRender'),
hook('bottom', 'init'), hook('bottom', 'didInitAttrs', { attrs: bottomAttrs }), hook('bottom', 'didReceiveAttrs', { newAttrs: bottomAttrs }), hook('bottom', 'willRender'),
hook('bottom', 'didInsertElement'), hook('bottom', 'didRender'),
hook('middle', 'didInsertElement'), hook('middle', 'didRender'),
hook('top', 'didInsertElement'), hook('top', 'didRender')
Expand Down Expand Up @@ -384,6 +386,30 @@ styles.forEach(style => {
component.destroy();
});
});

QUnit.test('properties set during `init` are availabe in `didReceiveAttrs`', function(assert) {
assert.expect(1);

registry.register('component:the-thing', style.class.extend({
init() {
this._super(...arguments);
this.propertySetInInit = 'init fired!';
},

didReceiveAttrs() {
this._super(...arguments);

assert.equal(this.propertySetInInit, 'init fired!', 'init has already finished before didReceiveAttrs');
}
}));

view = EmberView.extend({
template: compile(invoke('the-thing')),
container: container
}).create();

runAppend(view);
});
});

// TODO: Write a test that involves deep mutability: the component plucks something
Expand Down
7 changes: 7 additions & 0 deletions packages/ember-runtime/lib/system/core_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ import {
K
} from 'ember-metal/core';
import { validatePropertyInjections } from 'ember-runtime/inject';
import { symbol } from 'ember-metal/utils';

export let POST_INIT = symbol('POST_INIT');
var schedule = run.schedule;
var applyMixin = Mixin._apply;
var finishPartial = Mixin.finishPartial;
Expand Down Expand Up @@ -191,6 +193,8 @@ function makeCtor() {
this.init.apply(this, args);
}

this[POST_INIT]();

m.proto = proto;
finishChains(this);
sendEvent(this, 'init');
Expand Down Expand Up @@ -265,6 +269,9 @@ CoreObject.PrototypeMixin = Mixin.create({
@public
*/
init() {},

[POST_INIT]: function() { },

__defineNonEnumerable(property) {
Object.defineProperty(this, property.name, property.descriptor);
//this[property.name] = property.descriptor.value;
Expand Down
16 changes: 15 additions & 1 deletion packages/ember-views/lib/mixins/view_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { addObserver, removeObserver } from 'ember-metal/observer';
import { guidFor } from 'ember-metal/utils';
import { computed } from 'ember-metal/computed';
import { Mixin } from 'ember-metal/mixin';
import { POST_INIT } from 'ember-runtime/system/core_object';

import jQuery from 'ember-views/system/jquery';

Expand Down Expand Up @@ -609,14 +610,27 @@ export default Mixin.create({
this.scheduledRevalidation = false;

this._super(...arguments);
this.renderer.componentInitAttrs(this, this.attrs || {});

assert(
'Using a custom `.render` function is no longer supported.',
!this.render
);
},

/*
This is a special hook implemented in CoreObject, that allows Views/Components
to have a way to ensure that `init` fires before `didInitAttrs` / `didReceiveAttrs`
(so that `this._super` in init does not trigger `didReceiveAttrs` before the classes
own `init` is finished).
@method __postInitInitialization
@private
*/
[POST_INIT]: function() {
this._super(...arguments);
this.renderer.componentInitAttrs(this, this.attrs || {});
},

__defineNonEnumerable(property) {
this[property.name] = property.descriptor.value;
},
Expand Down

0 comments on commit 7315671

Please sign in to comment.