From 8a88851940cd69bb52480848e71638b745d8336a Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Thu, 14 Mar 2019 19:57:47 -0700 Subject: [PATCH 1/5] Add Injection Hook Normalization RFC --- text/0000-injection-hook-normalization.md | 302 ++++++++++++++++++++++ 1 file changed, 302 insertions(+) create mode 100644 text/0000-injection-hook-normalization.md diff --git a/text/0000-injection-hook-normalization.md b/text/0000-injection-hook-normalization.md new file mode 100644 index 0000000000..c3ddb82138 --- /dev/null +++ b/text/0000-injection-hook-normalization.md @@ -0,0 +1,302 @@ +- Start Date: 2019-03-14 +- Relevant Team(s): Ember.js, Ember Data, Learning +- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- Tracking: (leave this empty) + +# Injection Hook Normalization + +## Summary + +Standardize the basic injection hooks for Ember Octane's core framework objects: + +1. Glimmer Component +2. Service +3. Route +4. Controller +5. Helper + +This RFC would supercede the [Classic Class Owner Tunnel +RFC](https://github.com/emberjs/rfcs/pull/451) if it were to be accepted. + +## Motivation + +When the API design for Glimmer component finished up late last year, it was +decided that Glimmer component's `constructor` should receive the owner and +arguments directly as parameters, so injections and arguments would be available +to components during construction: + +```js +export default class MyComponent extends Component { + @service store; + + constructor(owner, args) { + super(owner, args); + + this.store.queryRecord('person', 123); + } +} +``` + +However, this has created a divergence with all other `EmberObject` based +classes, which do _not_ receive these values in the `constructor`, and instead +receive them just before `init` is triggered: + +```js +export default class Store extends Service { + @service fetch; + + constructor() { + super(); + + this.fetch; // this is undefined here + } + + init() { + super.init(); + + this.fetch.get('api/config'); // it's defined here instead + } +} +``` + +This divergence has brought up the more general question of how DI should assign +values during construction. Should we provide the `owner` to the constructor, +and require that users' clases set the owner themselves? Should we set the owner +after creation, and provide some kind of post-create hook? Should we try to +side-channel the owner somehow? + +After researching other DI frameworks such as Spring, Guice, Dagger, Weaver, +Typhoon, and Angular.js, we've found there are two common ways to inject values: + +1. Via explicit decoration of the constructor or the class. + + Spring: + + ```java + public class MovieRecommender { + + private final CustomerPreferenceDao customerPreferenceDao; + + @Autowired + public MovieRecommender(CustomerPreferenceDao customerPreferenceDao) { + this.customerPreferenceDao = customerPreferenceDao; + } + + // ... + } + ``` + + Dagger: + + ```java + class Thermosiphon implements Pump { + private final Heater heater; + + @Inject + Thermosiphon(Heater heater) { + this.heater = heater; + } + + ... + } + ``` + + Angular.js: + + ```js + import { Component } from '@angular/core'; + import { Hero } from './hero'; + import { HeroService } from './hero.service'; + + @Component({ + selector: 'app-hero-list', + template: ` +
+ {{hero.id}} - {{hero.name}} +
+ `, + }) + export class HeroListComponent { + heroes: Hero[]; + + constructor(heroService: HeroService) { + this.heroes = heroService.getHeroes(); + } + } + ``` + +2. Via decoration of properties, similar to Ember's current system: + + Dagger: + + ```java + class CoffeeMaker { + @Inject Heater heater; + @Inject Pump pump; + + ... + } + ``` + + Spring: + + ```java + public class MovieRecommender { + @Autowired + private MovieCatalog movieCatalog; + + // ... + } + ``` + +Importantly, _no_ injections occur without explicit configuration or decoration +of some kind in any of the most mature DI frameworks, and injected properties +are _only_ available post construction. Most frameworks also provide +[post-creation and pre-destruction][1] hooks for setting up the instance and +tearing it down after all injections have been assigned. + +[1]: https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#beans-postconstruct-and-predestroy-annotations + +This RFC proposes that we normalize the assignment of the owner, and access to +injected properties, to follow the same conventions as other popular DI +frameworks. Injections will be assigned _after_ objects have been constructed, +and a `didCreate` hook will be triggered after that. This will be +_approximately_ the same flow as `init` is currently for native classes, but is +a new hook to help with teaching purposes, since `init` has historically been +taught as the same as `constructor` in native classes (even though it is not). + +Doing this will ensure that users can rely on a consistent API for all DI +related classes, and that the lifecycle of the DI system will be teachable and +consisent. It also unblocks the usage of plain native classes which do not +extend a base class in the future. + +## Detailed design + +There are two parts to this design: + +1. **Glimmer Components** + + We'll update the `GlimmerComponent` class to _not_ receive any constructor + parameters. Instead, both `owner` and `args` will be assigned _after_ the + object has been fully constructed. We will also add the `didCreate` hook, + which will trigger immediately after `owner` and `args` have been assigned. + +2. **`EmberObject` based classes (Route/Controller/Service/Helper)** + + In the case of `EmberObject` classes, `didCreate` will trigger _after_ + `init`. Since the `owner` and arguments are assigned before `init`, it won't + be necessary to add any assignment code for these. + + Importantly, _only_ classes that are constructed by the container will have + `didCreate` triggered - utility classes that are made by extending + `EmberObject` and used throughout user code will only have `init`. This is + because the new hook is specifically for interacting with injections after + the class has been fully constructed, and does not make sense in classes that + are not initialized by the container. + +In the future these hooks could be generalized into an underlying set of +primitives that would allow users to declaratively specify injections and hooks +with decorators: + +```js +import { start, teardown } from "@ember/container"; +import { tracked } from "@glimmer/tracked" +import { glimmer } from "@glimmer/component" + +export default class ClockService { + time = Date.now(); + #interval = undefined; + + @start + start() { + #interval = setInterval(() => this.time = Date.now(), 1000) + } + + @tracked time; + + @teardown + destroy() { + clearInterval(#interval); + } +} + +export default @glimmer class MyComponent { + constructor(@service clock) { + this.createdAt = clock.time; + } +} +``` + +However, this is beyond the scope of this RFC. For now, the design is focussed +on staying in-line with this approach, and avoiding polluting the `constructor` +signatures for the various base classes provided in Ember. + +## How we teach this + +We would teach these concepts along with our material on DI in general. +`didCreate` and its counter-part, `willDestroy`, should be thought of as +container hooks that can be used to setup and teardown state based on +injections. + +An important point to cover will be the differences between `constructor` and +`didCreate`. We should teach `constructor` specifically as a mechanism for +_shaping_ the class, defining the basic fields and properties that will exist on +it. By contrast, `didCreate` is for setting up state within the larger system - +the Ember application. Concrete examples can be used to drive this point home. + +## Drawbacks + +- The previously proposed Glimmer component API has been built and highly + publicized. This change will definitely be surprising to some people, and may + cause confusion. However, v1.0.0 of Glimmer component has _not_ been published + yet, and we would not be breaking any semver guarantees. Additionally, there + are likely not many early adopters, since we have not yet marked it as stable. + +- There has been a lot of concern about confusion between `constructor` and + `init` as we head further down the native class adoption path. This decision + would formalize adopting `didCreate`, which is very similar to `init`, for + _all_ classes for the forseeable future. This could also cause confusion, + since container based classes would essentially always have this dichotomy of + two "creation" hooks. + + Creating a different hook with a new name should help with this issue. `init` + has historically been taught as the same as `constructor`, which is why some + users assume it will have the exact same semantics, and are confused when it + does not. Documenting the purpose for `didCreate`, along with clear, practical + examples for when to use it, and when to use `constructor`, should go a long + way toward preventing any additional confusion. + +## Alternatives + +- Standardizing on always passing the `owner` as the first parameter is another + option here. This would bring us inline with the current behavior of Glimmer + components, and a short term solution that would enable this is proposed in + the [Classic Class Owner Tunnel RFC](https://github.com/emberjs/rfcs/pull/451). + +- This RFC approaches the current problem directly - how do we address the + differences in behaviors of injections between different base classes in + Ember today, and what is the way we want users to interact with injections in + the long run? + + However, it is also touching on some larger underlying infrastructural debt. + The reason this has become an issue at all is, in part, because Ember's + container is heavily tied to the `EmberObject` model and its implementation + details. There are several ways we could move forward with disconnecting + these: + + 1. We could move the container in a more annotation based direction, as + described earlier in this RFC, and similarly to how other DI frameworks + work. + 2. We could implement a more generic Manager layer, similar to Component and + Modifier managers, that defines how a base class is managed by the + container. + 3. We could do some combination of these systems. + + This would be a much larger refactor, with much more design required, + especially if it would mean making more details of the container public. While + this would be good for updating and rationalizing Ember's internals in the + long run, these are implementation details that can still be addressed after + this decision has been made. It is not necessary to make these decisions now + to solve the basic question - should we pass injection parameters to the + `constructor` (without explicit annotation), or should we rely on lifecycle + hooks instead. From d7bab7e86d4cc9ca0c87237a168ba900494e2495 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Thu, 21 Mar 2019 16:04:56 -0700 Subject: [PATCH 2/5] update to decorators --- text/0000-injection-hook-normalization.md | 145 +++++++++------------- 1 file changed, 56 insertions(+), 89 deletions(-) diff --git a/text/0000-injection-hook-normalization.md b/text/0000-injection-hook-normalization.md index c3ddb82138..6facacee9c 100644 --- a/text/0000-injection-hook-normalization.md +++ b/text/0000-injection-hook-normalization.md @@ -160,89 +160,84 @@ tearing it down after all injections have been assigned. This RFC proposes that we normalize the assignment of the owner, and access to injected properties, to follow the same conventions as other popular DI frameworks. Injections will be assigned _after_ objects have been constructed, -and a `didCreate` hook will be triggered after that. This will be +and a `AFTER_INJECTION` symbol hook will be triggered after that. This will be _approximately_ the same flow as `init` is currently for native classes, but is a new hook to help with teaching purposes, since `init` has historically been taught as the same as `constructor` in native classes (even though it is not). +We will also trigger a `BEFORE_DESTRUCTION` symbol hook when the container is +cleaning up objects. Finally, we'll create decorators that users can use to +decorate functions to run during these lifecycle hooks, so users have a friendly +and flexible way to define their class's lifecycles. Doing this will ensure that users can rely on a consistent API for all DI related classes, and that the lifecycle of the DI system will be teachable and consisent. It also unblocks the usage of plain native classes which do not -extend a base class in the future. +extend a base class in the future. Finally , it will allow us to continue to +build on the DI system, and ## Detailed design -There are two parts to this design: +There are three parts to this design: -1. **Glimmer Components** +### Symbols and Decorators - We'll update the `GlimmerComponent` class to _not_ receive any constructor - parameters. Instead, both `owner` and `args` will be assigned _after_ the - object has been fully constructed. We will also add the `didCreate` hook, - which will trigger immediately after `owner` and `args` have been assigned. +Two symbols will be added to Ember: -2. **`EmberObject` based classes (Route/Controller/Service/Helper)** +- `AFTER_INJECTION` +- `BEFORE_DESTRUCTION` - In the case of `EmberObject` classes, `didCreate` will trigger _after_ - `init`. Since the `owner` and arguments are assigned before `init`, it won't - be necessary to add any assignment code for these. +These symbols will be publicly accessible, and will be the keys for the methods +that should be called on objects at the respective points in their lifecycles. +We will also add two decorators: - Importantly, _only_ classes that are constructed by the container will have - `didCreate` triggered - utility classes that are made by extending - `EmberObject` and used throughout user code will only have `init`. This is - because the new hook is specifically for interacting with injections after - the class has been fully constructed, and does not make sense in classes that - are not initialized by the container. +- `@afterInjection` +- `@beforeDestruction` -In the future these hooks could be generalized into an underlying set of -primitives that would allow users to declaratively specify injections and hooks -with decorators: +These decorators will be usable on methods, and will cause the methods to run +after injection and before destruction. Multiple methods can be decorated, and +will all run in the order that decorators are evaluated (from top to bottom in +the stage 1 spec). -```js -import { start, teardown } from "@ember/container"; -import { tracked } from "@glimmer/tracked" -import { glimmer } from "@glimmer/component" +### Glimmer Components -export default class ClockService { - time = Date.now(); - #interval = undefined; +We'll update the `GlimmerComponent` class to _not_ receive any constructor +parameters. Instead, both `owner` and `args` will be assigned _after_ the object +has been fully constructed. We will also add the `AFTER_INJECTION` hook, which +will trigger immediately after `owner` and `args` have been assigned. The +`willDestroy` hook will also be renamed to `BEFORE_DESTRUCTION`. No default +implementation will be provided on the base class. - @start - start() { - #interval = setInterval(() => this.time = Date.now(), 1000) - } +### `EmberObject` based classes (Route/Controller/Service/Helper) - @tracked time; +In the case of `EmberObject` classes, `AFTER_INJECTION` will trigger _after_ +`init`. Since the `owner` and arguments are assigned before `init`, it won't be +necessary to add any assignment code for these. - @teardown - destroy() { - clearInterval(#interval); - } -} +Importantly, _only_ classes that are constructed by the container will have +`AFTER_INJECTION` triggered - utility classes that are made by extending +`EmberObject` and used throughout user code will only have `init`. This is +because the new hook is specifically for interacting with injections after the +class has been fully constructed, and does not make sense in classes that are +not initialized by the container. -export default @glimmer class MyComponent { - constructor(@service clock) { - this.createdAt = clock.time; - } -} -``` - -However, this is beyond the scope of this RFC. For now, the design is focussed -on staying in-line with this approach, and avoiding polluting the `constructor` -signatures for the various base classes provided in Ember. +`BEFORE_DESTRUCTION` will trigger `destroy` in EmberObjects by default, since +the function currently runs at the same time. Users will be able to override +this function and call the super function using `super[BEFORE_DESTRUCTION]` or +`this._super()` in classic classes. ## How we teach this -We would teach these concepts along with our material on DI in general. -`didCreate` and its counter-part, `willDestroy`, should be thought of as -container hooks that can be used to setup and teardown state based on -injections. +We would teach these concepts along with our material on DI in general. We +primarily want to teach the `@afterInjection` and `@beforeDestruction` +decorators for native classes, and then teach the existence of the symbols for +classic class users. An important point to cover will be the differences between `constructor` and -`didCreate`. We should teach `constructor` specifically as a mechanism for -_shaping_ the class, defining the basic fields and properties that will exist on -it. By contrast, `didCreate` is for setting up state within the larger system - -the Ember application. Concrete examples can be used to drive this point home. +`@afterInjection` methods. We should teach `constructor` specifically as a +mechanism for _shaping_ the class, defining the basic fields and properties that +will exist on it. By contrast, methods decorated with `@afterInjection` are for +setting up state within the larger system - the Ember application. Concrete +examples can be used to drive this point home. ## Drawbacks @@ -254,17 +249,17 @@ the Ember application. Concrete examples can be used to drive this point home. - There has been a lot of concern about confusion between `constructor` and `init` as we head further down the native class adoption path. This decision - would formalize adopting `didCreate`, which is very similar to `init`, for - _all_ classes for the forseeable future. This could also cause confusion, + would formalize adopting `@afterInjection`, which is very similar to `init`, + for _all_ classes for the forseeable future. This could also cause confusion, since container based classes would essentially always have this dichotomy of two "creation" hooks. Creating a different hook with a new name should help with this issue. `init` has historically been taught as the same as `constructor`, which is why some users assume it will have the exact same semantics, and are confused when it - does not. Documenting the purpose for `didCreate`, along with clear, practical - examples for when to use it, and when to use `constructor`, should go a long - way toward preventing any additional confusion. + does not. Documenting the purpose for `@afterInjection`, along with clear, + practical examples for when to use it, and when to use `constructor`, should + go a long way toward preventing any additional confusion. ## Alternatives @@ -272,31 +267,3 @@ the Ember application. Concrete examples can be used to drive this point home. option here. This would bring us inline with the current behavior of Glimmer components, and a short term solution that would enable this is proposed in the [Classic Class Owner Tunnel RFC](https://github.com/emberjs/rfcs/pull/451). - -- This RFC approaches the current problem directly - how do we address the - differences in behaviors of injections between different base classes in - Ember today, and what is the way we want users to interact with injections in - the long run? - - However, it is also touching on some larger underlying infrastructural debt. - The reason this has become an issue at all is, in part, because Ember's - container is heavily tied to the `EmberObject` model and its implementation - details. There are several ways we could move forward with disconnecting - these: - - 1. We could move the container in a more annotation based direction, as - described earlier in this RFC, and similarly to how other DI frameworks - work. - 2. We could implement a more generic Manager layer, similar to Component and - Modifier managers, that defines how a base class is managed by the - container. - 3. We could do some combination of these systems. - - This would be a much larger refactor, with much more design required, - especially if it would mean making more details of the container public. While - this would be good for updating and rationalizing Ember's internals in the - long run, these are implementation details that can still be addressed after - this decision has been made. It is not necessary to make these decisions now - to solve the basic question - should we pass injection parameters to the - `constructor` (without explicit annotation), or should we rely on lifecycle - hooks instead. From cd49e50b72ce04a8da736459741b9cbf485e832c Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Thu, 21 Mar 2019 16:07:38 -0700 Subject: [PATCH 3/5] RFC link --- text/0000-injection-hook-normalization.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-injection-hook-normalization.md b/text/0000-injection-hook-normalization.md index 6facacee9c..5d4d6a8b3c 100644 --- a/text/0000-injection-hook-normalization.md +++ b/text/0000-injection-hook-normalization.md @@ -1,6 +1,6 @@ - Start Date: 2019-03-14 - Relevant Team(s): Ember.js, Ember Data, Learning -- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- RFC PR: https://github.com/emberjs/rfcs/pull/467 - Tracking: (leave this empty) # Injection Hook Normalization From d823bc17e20ed2bf3bf71592246346795f38847b Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 22 Mar 2019 13:50:44 -0700 Subject: [PATCH 4/5] add some examples --- text/0000-injection-hook-normalization.md | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/text/0000-injection-hook-normalization.md b/text/0000-injection-hook-normalization.md index 5d4d6a8b3c..c976aef0d2 100644 --- a/text/0000-injection-hook-normalization.md +++ b/text/0000-injection-hook-normalization.md @@ -188,6 +188,19 @@ Two symbols will be added to Ember: These symbols will be publicly accessible, and will be the keys for the methods that should be called on objects at the respective points in their lifecycles. + +```js +class Profile extends Component { + [AFTER_INJECTION]() { + // setup component... + } + + [BEFORE_DESTRUCTION]() { + // teardown component... + } +} +``` + We will also add two decorators: - `@afterInjection` @@ -198,6 +211,25 @@ after injection and before destruction. Multiple methods can be decorated, and will all run in the order that decorators are evaluated (from top to bottom in the stage 1 spec). +```js +class Profile extends Component { + @afterInjection + setup() { + // setup component... + } + + @afterInjection + fetchData() { + // fetch data... + } + + @beforeDestruction + teardown() { + // teardown component... + } +} +``` + ### Glimmer Components We'll update the `GlimmerComponent` class to _not_ receive any constructor From e373f2081a50a514cf1737d020378c90bdc72736 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 22 Mar 2019 16:22:07 -0700 Subject: [PATCH 5/5] update examples, and learning --- text/0000-injection-hook-normalization.md | 126 ++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/text/0000-injection-hook-normalization.md b/text/0000-injection-hook-normalization.md index c976aef0d2..97f7480324 100644 --- a/text/0000-injection-hook-normalization.md +++ b/text/0000-injection-hook-normalization.md @@ -190,6 +190,8 @@ These symbols will be publicly accessible, and will be the keys for the methods that should be called on objects at the respective points in their lifecycles. ```js +import { AFTER_INJECTION, BEFORE_DESTRUCTION } from '@ember/di'; + class Profile extends Component { [AFTER_INJECTION]() { // setup component... @@ -212,6 +214,8 @@ will all run in the order that decorators are evaluated (from top to bottom in the stage 1 spec). ```js +import { afterInjection, beforeDestruction } from '@ember/di'; + class Profile extends Component { @afterInjection setup() { @@ -230,6 +234,8 @@ class Profile extends Component { } ``` +These new APIs will be importable from `@ember/di`; + ### Glimmer Components We'll update the `GlimmerComponent` class to _not_ receive any constructor @@ -271,6 +277,126 @@ will exist on it. By contrast, methods decorated with `@afterInjection` are for setting up state within the larger system - the Ember application. Concrete examples can be used to drive this point home. +Example docs (this comes after introducing services): + +### Container Lifecycle Hooks + +As we've discussed before, Ember.js uses _dependency injection_ to manage +the various instances of framework classes such as components, services, routes, +and controllers. In this setup, Ember apps create class instances automatically +as they are used, and register and manage them internally. This is how we're +able to _inject_ values, such as services in classes - we look up where they are +registered, and then pass them into classes that ask for them: + +```js +import Component from '@glimmer/component'; +import { inject as service } from '@ember/service'; + +export default class Chat extends Component { + // By decorating this property, we're telling Ember.js + // that when it creates a Profile component, it should + // lookup the "messages" service, and assign it to the + // messages field on the component. + @service messages; +} +``` + +Some classes require additional setup after they've been created and have access +to these injections, and some require teardown before they're destroyed. +However, injections are not available during the `constructor` for objects: + +```js +import Component from '@glimmer/component'; +import { inject as service } from '@ember/service'; + +export default class Chat extends Component { + @service messages; + + constructor() { + super(); + + this.messages.subscribe(this.args.chatId); + } +} +``` + +And there is no opposite of a `constructor` method built into JavaScript +classes. Instead, Ember provides the `@afterInjection` and `@beforeDestruction` +decorators to allow you to decorate methods and tell Ember that they should be +run at these points in the class lifecycle. + +```js +import Component from '@glimmer/component'; +import { inject as service } from '@ember/service'; +import { afterInjection, beforeDestruction } from '@ember/di'; + +export default class Chat extends Component { + @service messages; + + @afterInjection + subscribe() { + this.messages.subscribe(this.args.chatId); + } + + @beforeDestruction + unsubscribe() { + this.messages.unsubscribe(this.args.chatId); + } +} +``` + +The `afterInjection` hook will run immediately after _all_ injections have been +assigned to the class, meaning you can run any setup code you want with them, +and `beforeDestruction` will run just before Ember removes the class. Multiple +methods can also be decorated with either decorator: + +```js +import Component from '@glimmer/component'; +import { inject as service } from '@ember/service'; +import { afterInjection, beforeDestruction } from '@ember/di'; + +export default class Chat extends Component { + @service messages; + @service notifications; + + @afterInjection + subscribeToMessages() { + this.message.subscribe(this.args.chatId); + } + + @beforeDestruction + unsubscribeFromMessages() { + this.message.unsubscribe(this.args.chatId); + } + + @afterInjection + subscribeToNotifications() { + this.notifications.subscribe(this.args.chatId); + } + + @beforeDestruction + unsubscribeFromNotifications() { + this.notifications.unsubscribe(this.args.chatId); + } +} +``` + +#### `constructor` vs `@afterInjection` + +If Ember classes are not fully initialized during the `constructor`, you may be +wondering what it's usefulness is. The main purpose of the constructor is for +setting up the _initial state_ of the class, before it interacts with any other +parts of the application. + +Class fields and the constructor both run during this phase, and they give your +class a _shape_ that makes it much easier for the browser to optimize and +understand your class. Types of values you should initialize in `constructor` +and class fields include: + +- All properties that will ever exist on the object +- Objects/arrays +- Class instances + ## Drawbacks - The previously proposed Glimmer component API has been built and highly