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

[BUGFIX release-1-13] Do not require _super in didRecieveAttrs. #12138

Merged

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 18, 2015

Default framework hooks should not require that users call this._super (however, it is still a best practice).

This adds a new _internalDidReceiveAttrs hook for use internally to avoid using a didRecieveAttrs in AttrsProxy.

Fixes #11992.

Default framework hooks should not *require* that users call
`this._super` (however, it is still a best practice).

This adds a new `_internalDidReceiveAttrs` hook for use internally to
avoid using a `didRecieveAttrs` in `AttrsProxy`.
rwjblue added a commit that referenced this pull request Aug 18, 2015
…eceive-attrs

[BUGFIX release-1-13] Do not require _super in didRecieveAttrs.
@rwjblue rwjblue merged commit 981e3ed into emberjs:master Aug 18, 2015
@rwjblue rwjblue deleted the avoid-requiring-super-in-did-receive-attrs branch August 18, 2015 20:15
@poteto
Copy link

poteto commented Aug 27, 2015

In case anyone wants to know, it is good practice because:

@rwjblue
Imagine you have a component that includes a couple mixins or extends from a shared base class. That component may implement willDestroy, didInsertElement, etc. Later if you ever add those to your parent class or shared mixins, you will have issues if you didn't call _super. So, it is much easier to just have as a rule of thumb: "always call super for hooks". It should absolutely not be required (hence the fixes in 1.13.9), but without calling super you have a major refactoring hazard.

@ultimatemonty
Copy link

@rwjblue @poteto quick question related to this.

If I have a base component that hooks into didReceiveAttrs via Ember.on('didReceiveAttrs') and then extend that base component do I need to called this._super(); if I also hook into didReceiveAttrs in the extended component?

// base component
Ember.Component.extend({
  Ember.on('didReceiveAttrs', function() {
    // do some awesome stuff
  })
});

// extends base component
BaseComponent.extend({
  Ember.on('didReceiveAttrs', function() {
    // awesome stuff specific to this component
    // do I need to call this._super() here if I need to use the BaseComponent functionality as well?
  })
});

@poteto
Copy link

poteto commented Sep 18, 2015

No, you only need to call super when overriding an existing method on a parent class. Using on listens for the event to be triggered, which doesn't override anything (unless the method name you give it does!)

E.g.

BaseComponent.extend({
  doStuff: Ember.on('didReceiveAttrs', function() {
    // no super call required, as `doStuff` is not a method on the parent class
  }),

  didInsertElement() {
    this._super(...arguments); // overriding an existing method on the parent class
  },

  willDestroyElement: Ember.on(/* ... */) // don't do this
});

// extends from BaseComponent
ChildComponent.extend({
  doStuff() {
    this._super(...arguments); // if you want to preserve BaseComponent's `doStuff` method
  }
});

@rwjblue
Copy link
Member Author

rwjblue commented Sep 18, 2015

Thank you @poteto!

@ultimatemonty
Copy link

what @rwjblue said!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants