diff --git a/text/0000-native-class-constructor-update.md b/text/0000-native-class-constructor-update.md new file mode 100644 index 0000000000..e44ea996c4 --- /dev/null +++ b/text/0000-native-class-constructor-update.md @@ -0,0 +1,247 @@ +- Start Date: 2018-06-14 +- RFC PR: (leave this empty) +- Ember Issue: (leave this empty) + +# Native Class Constructor Update + +## Summary + +Update the behavior of EmberObject's constructor to defer object +initialization. + +## Motivation + +Using native class syntax with EmberObject has almost reached full feature +parity, meaning soon we'll be able to ship native classes and begin recommending +them. This will do wonders for the Ember learning story, and will bring us in +line with the wider Javascript community. + +However, early adopters of native classes have experienced some serious +ergonomic issues due to the current behavior of the class constructor. The issue +is caused by the fact that properties passed to `EmberObject.create` are +assigned to the instance in the root class `constructor`. Due to the way that +native class fields work, this means that they are assigned _before_ any +subclasses' fields are assigned, causing subclass fields to overwrite any value +passed to `create`: + +```js +class Foo extends EmberObject { + bar = 'baz'; +} + +let foo = Foo.create({ bar: 'something different' }); + +console.log(foo.bar); // 'baz' +``` + +This has made adoption very difficult, and is a consistent stumbling block for +new users of native class syntax in Ember. Worse yet, it makes writing a codemod +for converting to native class syntax very difficult because we don't have a +clear target. + +For instance, given the above class, how would we convert the class field? Let's +go through the various options: + +```js +class Foo extends EmberObject { + // Does not work, for the reasons described above + bar = 'baz'; + + // Does not cover all cases. If we did `Foo.create({ bar: false })` it would + // still assign the default. + bar = this.bar || 'baz'; + + // This works, but is very verbose and not ideal + bar = this.hasOwnProperty('bar') ? this.bar : 'baz'; + + // This is one of the community accepted solutions, but it requires lodash + bar = _.defaultTo(this.bar, 'baz'); + + // This is another community accepted solution, but it requires + // @ember-decorators/argument, which is a separate library + @argument foo = 'bar'; +} +``` + +None of these is ideal. Instead, we can change the behavior of the constructor +and the `create` method to circumvent this issue. + +This change _would_ be a breaking change to the behavior of native classes +today, and a change from the previous class RFC. This will impact early adopters +and should be made with that in mind. It would _not_ be a change that breaks the +behavior of the community solutions to class fields mentioned above, and all +other changes would be relatively easy to create a safe codemod for (essentially +converting `constructor` -> `init` in affected classes), so the impact _should_ +be minimal. + +Because native classes never officially shipped as part of Ember's public API +(an announcement was not made, docs have not been written, etc), this RFC +proposes that the change would _not_ be considered a breaking change _for the +purposes of semver_. This would allow us to ship the change during the Ember v3 +release cycle, and prevent more code from being built on top of the previous +behavior. + +## Detailed design + +One very important design constraint to making this change is that we _cannot_ +break the behavior of EmberObject when used _without_ native classes. To do +this, we will leverage the fact that the static `create` method is the only +public way to create an instance of EmberObject. + +Currently, the behavior of EmberObject is the following (simplified): + +```js +class EmberObject { + constructor(props) { + // ..class setup things + + Object.assign(this, props); + this.init(); + } + + static create(props) { + let instance = new this(props); + + return instance; + } +} +``` + +We can change it to the following (simplified): + +```js +class EmberObject { + constructor(props) { + // ..class setup things + } + + static create(props) { + let instance = new this(props); + + Object.assign(instance, props); + instance.init(); + + return instance; + } +} +``` + +This would assign the properties _after_ all of the class fields for any +subclasses have been assigned. Revisiting our previous example, the following +two class declarations would effectively be equivalent: + +```js +const Foo = EmberObject.extend({ + bar: 'baz' +}); + +class Foo extends EmberObject { + bar = 'baz'; +} +``` + +Much easier to codemod! There are other subtle differences between native class +fields and EmberObject properties, such as the fact that class fields are +assigned each time a class is initialized, but these are easier to work around. + +### Injections and the `init` hook + +One side effect of this change is that injections will not be available on the +class instance during the `constructor` phase. This behavior is not very +commonly used - based on an informal community survey we found only a few usages +\- but it _does_ exist and have its use cases. + +Figuring out the ideal behavior of injections during the constructor phase is +outside of the scope of this RFC, and is something that should be discussed in +future RFCs. For the time being, users can still rely on the `init` hook, which +will continue to be called after all injections and properties have been +assigned to the instance. + +### `new EmberObject()` + +It was previously possible to use `new` syntax with EmberObject. While this +was not considered public API, it has technically worked and been under test +since the early days of Ember, and may fall under the category of intimate API. +Ideally, we would deprecate this usage as a private/intimate API, which would +mean supporting it through the next LTS version, and dropping support after +(currently, this would mean dropping it at `v3.5.0`). + +We can continue to support this behavior in a backwards compatible way while +deprecating it with one final tweak to the change above: + +```js +const DEFER_INIT = new Symbol(); + +function initialize(instance, props) { + Object.assign(instance, props); + instance.init(); +} + +class EmberObject { + constructor(props, maybeDefer) { + // ..class setup things + + if (maybeDefer === DEFER_INIT) { + return this; + } + + deprecate('using `new` with EmberObject has been deprecated. Please use `create` instead.', false, { + id: 'object.new-constructor', + until: '3.5.0' + }); + + initialize(this, props); + } + + static create(props) { + let instance = new this(props, DEFER_INIT); + initialize(instance, props); + + return instance; + } +} +``` + +## How we teach this + +If this PR is accepted, most of the major issues with classes will have been +resolved. We can begin working on a codemod to make converting easier, and move +toward officially making native classes a finalized part of the public API of +Ember. Pending decorators and class fields moving to a late enough stage in the +TC39 process, we can also begin converting the guides to use native class +syntax. + +We can document the exact behavior of the new constructor in the API docs for +EmberObject. Most details won't have to change since this change only affects +native class syntax, which has not been documented much officially. We can also +demonstrate the behaviors of classes throughout the guides and API docs. + +One thing we should make clear is that EmberObject will likely be deprecated +in the near future, and that ideally for non-Ember classes (things that aren't +Components, Services, etc.) users should drop EmberObject altogether and use +native classes only. + +## Drawbacks + +This would be a breaking change that could negatively affect early adopters. + +## Alternatives + +* We could leave the behavior as is, and choose a method for defaulting to +standardize on. + +* We could make this change behind a feature flag and require users to opt-in +to the new behavior, like optional features that currently exist. This would +have to be a build time feature flag, since the area is very performance +sensitive. Given native classes are not yet public API, if we were to do this we +should probably still default to enabling the new behavior and recommending it +as the preferred path. + +* We could not deprecate `new EmberObject` altogether, and instead only +deprecate passing properties to the constructor. While this would work as a +temporary solution, it may also encourage users to continue using EmberObject +instead of switching to native classes, which is ultimately the long term goal. + +## Unresolved questions + +How do we handle DI during the construction phase?