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] Adds default implementations of Component lifecycle hooks #17169

Merged

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Oct 31, 2018

Adds default implementations for all of Ember.Component's lifecycle hooks. Fixes #17156.

There are some notable concerns here:

  1. Performance, default implementations means that we will be doing slightly more work in cases where users are calling _super in a hook which did not have a default implementation. This may be worth benchmarking, but I'm unsure if it would make that much of a difference.
  2. The didDestroyElement hook was not previously documented, and may not have been considered public API. It is under test, and there are no internal usages of it besides test, so it may be a better idea to not publicly expose it and deprecate/remove it instead.

The alternative would be to remove lifecycle hook defaults entirely. As long as we're consistent in telling users "always call super" or "never call super unless its your own code" I think it would be fine. The current situation, where we sometimes-do-and-sometimes-don't, is the worst of both worlds. Removing lifecycle hooks entirely may require a deprecation.

@pzuraq pzuraq mentioned this pull request Oct 31, 2018
24 tasks
@pzuraq
Copy link
Contributor Author

pzuraq commented Nov 1, 2018

cc @rwjblue

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.

None yet

2 participants