Deprecate the usage of Ember.on for Component's lifecycle events.

[givanse]

Request

Deprecate this:

Ember.Component.extend({
  someRandomName: Ember.on('didInsertElement', function() {
  })
});

In favor of:

Ember.Component.extend({
  didInsertElement: function() {
    this._super(...arguments);
  }
});

Reasoning

Ember.Component.extend({
  doA: Ember.on('willInsertElement', function() { },
  doB: Ember.on('willInsertElement', function() { }
})

The order in which doA and doB happens is unknown. This code might be the result of a refactoring. Or, a developer new to the code base not noticing that the event is being observed already. Subtle bugs can be easily introduced.

It is also easier to read and understand the behavior of a component if there is a single place where all the event dependent code is executed. Tracking down all the different pieces of code that are executed at different moments in the lifecycle is cumbersome and can be easily avoided by using overrides.

One argument that might come up against overrides is:

what if people forgets to call super? that can introduce bugs too!

It is easier to know that you always have to have that call inside the hook, there is determinism. Whereas keeping track of all the Ember.ons that might be cohabiting an object is not straightforward. When reading the code, you have to keep a mental map of all the pieces that might be executed by a hook; the cognitive load is high.

[stefanpenner]

and all the on hooks are slow

[mmun]

Another point against the Ember.on style is that you are forced to choose a key name (doA, doB) for each of the event handlers.

[givanse]

Since on hooks are slow, should the scope be to completely deprecate Ember.on? Doesn't seem to be used that much:

cd emberjs/ember.js
grep -R -e '\.on(' packages/ | wc -l
47
# a couple less than that, the regex is not perfect

Not assuming that is trivial, but it doesn't seem to be all over the place either.

[locks]

@givanse which packages are this? Ember.on, for framework hook methods, is widely used in applications in the wild so I'd rather the events that have hook method alternatives to be deprecated.

[givanse]

It was emberjs/ember.js/packages. (updated the snippet)

Ember.on can be removed by its users over time, just as views were removed. A deprecation warning can be put in place for as long as needed. Aside from wide spread usage what else can be said in favor of .on?

[locks]

@givanse it's how you listen to Ember events :P

[stefanpenner]

@givanse which packages are this? Ember.on, for framework hook methods, is widely used in applications in the wild so I'd rather the events that have hook method alternatives to be deprecated.

Yes, I would really like to get critical mass on-board with moving away from these. We burn many extra CPU cycles speculatively trigging a huge number of events during both ember-data models, and ember component lifecycles. Although we do also speculative call the method hooks, those atleast perform much better then the event listener equivalents.

In addition to the perf overhead, the ordering/extended traps these have over just method + super cause real pain, not to mention yet another thing to teach.

[cibernox]

FWIW, I've ditched Ember.on a long time ago and I fully support this.

The transition to ES6 classes can be the perfect time, so if glimmer-components includes the changes in the ObjectModel as part of it, I'd push to also use that transition as an opportunity to remove this.

[DingoEatingFuzz]

I'm apparently not in the majority here, but I like .on. Even though, as @mmun points out, you have to choose a key name, I find that far better than having to use this._super. Case and point, there is a bug in your sample code, @givanse:

Ember.Component.extend({
  didInsertElement: function() {
    this._super(...arguments); // You need to return here
  }
});

Without returning the result of this._super, you aren't properly overriding that method. This isn't an issue with the .on style. As for the sequence in which multiple .ons fire, the trivial solution is to put imperative code in a single listener.

My opinions here aren't strong, but this thread was in need of a counter-argument.

[DingoEatingFuzz]

For didInsertElement in particular, you don't, but generally speaking, the super may return something other than undefined. And even if the super doesn't return today, it could in the future. This is more prevalent in Ember Data.

[DingoEatingFuzz]

@locks How is that relevant to general commentary on method overriding?

[locks]

Whoops, I saw my comments repeated and deleted one of them. They weren't repeated after all.

[AKST]

As a user who up to this point has avoided overriding lifecycle hooks, I was unaware the use of Ember.on was that bad, I always consider it the safer approach as you didn't have to always remember to call this._super plus I found the super call kind of unergonomic, so I've always recommended people use Ember.on instead. This was mostly because I was bitten by this early on.

Is it worth pursing a more ergonomic alternative to overriding the methods and calling super?

[kkincade]

@stefanpenner when you say the on hooks are slow, have you quantified this? I'm curious to know how much slower the on hooks are from the hook method alternative.