-
-
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 beta] Ensure that this._super
is called by Components.
#12396
Conversation
@@ -7,6 +7,9 @@ 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 { symbol } from 'ember-metal/utils'; | |||
|
|||
let INIT_WAS_CALLED = symbol('INIT_WAS_CALLED'); |
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.
Opportunity for const
.
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.
Good point, updated!
4babaea
to
e239b48
Compare
@@ -610,6 +613,7 @@ export default Mixin.create({ | |||
this.scheduledRevalidation = false; |
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.
although not part of this commit, should this assignment to this
be before the init
?
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 not sure I follow what you mean. This assignment is happening just before this._super
is called, do you want to move it afterwards or something 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.
yes, as in ES6 you can't touch this
before calling super, we should slowly move our internals to this.
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 to call this._super
first (before using anything on this
).
LGTM |
e239b48
to
615d79e
Compare
When implementing `init` in a component subclass you must call `this._super(...arguments)`. If you do not you will get an error. Unfortunately, that error is **very** hard to understand and is completely unrelated to code that you have written. This change adds a small flag (via symbol to avoid exposing this publicly) that we can use to ensure that `_super` was called properly. --- After these changes, given the following: ```javascript import Ember from 'ember'; export default Ember.Component.extend({ init() { this.doThings(); } }); ``` Will throw an error that explains that you must call `_super`: ``` You must call `this._super(...arguments);` when implementing `init` in a component. Please update ${this} to call `this._super` from `init`. ```
615d79e
to
bf455fb
Compare
You should generally always call `this._super(...arguments);` before touching `this` in the constructor. This requirement is actually enforced by ES2015 classes...
[BUGFIX beta] Ensure that `this._super` is called by Components.
When implementing
init
in a component subclass you must callthis._super(...arguments)
. If you do not you will get an error. Unfortunately, that error is very hard to understand and is completely unrelated to code that you have written.This change adds a small flag (via symbol to avoid exposing this publicly) that we can use to ensure that
_super
was called properly.After these changes, given the following:
Will throw an error that explains that you must call
_super
:/cc @ef4