-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BUGFIX release] Ensure init
is completed before didReceiveAttrs
fires.
#12260
Conversation
@@ -191,6 +191,10 @@ function makeCtor() { | |||
this.init.apply(this, args); | |||
} | |||
|
|||
if (this.__postInitInitialization) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just call it __postInit
post initialization initialization is a bit redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, can we make this a symbol ? That way no user can use it, and it wont collide with anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm slightly concerned that the property miss will not be as ideal as we would want, although I suspect today it is just in the noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will:
- implement as a symbol (we can always tweak later)
- add to base object to avoid property lookup (then remove
if
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
I'm unclear how this will work in ES6 class land. Unless keep init around, then something like:
|
…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.
778054a
to
7315671
Compare
This was something that @mixonic also mentioned, and is something we should definitely decide on before merging this. I was under the impression that the idea that For additional context there are a few things that currently happen after |
@rwjblue ya i think we don't have a choice but to keep the two |
We discussed this in the 2015-09-04 core team meeting. In general it seems that we had planned to have I would still like to hear from @mixonic before landing. |
@private | ||
*/ | ||
[POST_INIT]: function() { | ||
this._super(...arguments); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh, I would probably leave out the _super
as it is a hook ;-)
[BUGFIX release] Ensure `init` is completed before `didReceiveAttrs` fires.
Given the following example:
Currently, the
didReceiveAttrs
(and alsodidInitAttrs
) is firing from within thethis._super()
call ininit
(because we trigger these hooks inEmber.View
'sinit
). Which means thatdidReceiveAttrs
can not have access to things set ininit
.This change allows us to avoid that circumstance, and ensure that
init
is fully completed beforedidReceiveAttrs
is fired and also ensures tahtdidReceiveAttrs
/didInitAttrs
is still fired from within theconstructor
before observation is started.