diff --git a/text/0000-es-classes.md b/text/0000-es-classes.md new file mode 100644 index 0000000000..fa4b5ff3f3 --- /dev/null +++ b/text/0000-es-classes.md @@ -0,0 +1,395 @@ +- Start Date: 2017-07-28 +- RFC PR: (leave this empty) +- Ember Issue: (leave this empty) + +# Summary + +This RFC aims to solidify the usage of ES2015 Classes as a public API of Ember so +that users can begin building on them, and projects like `ember-decorators` can +continue to push forward with experimental Javascript features. This includes: + +* Making the class `constructor` function a public API +* Modifying some of the internals of `Ember.Object` to support existing features +and make the usage of ES Classes cross-compatible with `Ember.Object` + +It does _not_ propose additions in the form of helpers or decorators, which should +continue to be iterated on in the community as the spec itself is finalized. It also +does not propose deprecating or removing existing functionality in `Ember.Object`. + +# Motivation + +The Ember Object model has served its purpose well over the years, but now that +ES Classes are becoming prevalent throughout the wider Javascript community +it is beginning to show its age. With class properties at stage 3 and decorators at +stage 2 in the TC39 process, classes are finally at a point where we can start +integrating them into Ember. + +The [ember-decorators](https://github.com/rwjblue/ember-decorators) project has been +experimenting with using ES Classes and filling out the Ember feature-set, +allowing us to write Ember classes like so: + +```javascript +export default class MyComponent extends Ember.Component { + didInsertElement() { + // do stuff + } + + @computed + get foo() { + // do stuff + } + + @action + bar() { + // do stuff + } +} +``` + +Using classes makes Ember easier to teach and understand by normalizing it with +standard Javascript coding practices, and allows us to share code and solutions +with other frameworks and libraries. It also brings with it all the benefits of +ES Class syntax: + +* More aligned with the greater Javascript community +* Ability to share code more easily with other libraries and frameworks +* Easier to statically analyze +* Cleaner and easier to read (subjective) + +The Ember Object model already works extremely well with ES classes, as +demonstrated above, but there several failure scenarios. Furthermore, because +they are not officially supported as a public API, there is no guarantee +that they will continue to work well. Thus, this RFC seeks to solidify the +behavior of ES Classes so that the community can continue to experiment with +new Javascript features and build on a stable API. + +# Detailed Design + +Many of the standard features of Ember classes work out of the box today, either with +vanilla ES Classes or through `ember-decorators`, including: + +* Inheritance +* Lifecycle hooks +* Computeds +* Injections +* Actions + +However, the following features either do not exist or do not work as a +user familiar with `Ember.Object` would expect: + +* Extending from ES Classes using `extend` +* Class properties +* Mixins +* Observers and events +* Merged and concatenated properties + +These features will require changes to `Ember.Object` + +## Extend + +Currently, once a class is defined using ES Classes it is not possible for users +to extend it using the previous CoreObject style of writing and extending classes. +This can limit the rate of adoption because ES Classes would become a trapdoor - +once you begin using them, you must continue to use them. It would be a particularly +thorny issue for addon developers, who may design components which their users +expect to be able to extend and modify. + +This RFC proposes that `extend` be fixed on ES Classes to make them fully +cross-compatible with the existing syntax. There are two general approaches to +making this work: + +1. Modify CoreObject to use prototypes/ES Classes internally. This would bring +CoreObject more inline with ES Classes, but would be a significant internal change. + +2. Modify CoreObject to have different behavior if it is extending an ES Class +using `extend`. + +Both approaches should be explored and benchmarked to determine if there are an +significant advantages to one over the other. + +## Class Properties + +When using `Ember.Object.extend`, properties that are passed in on the object +are assigned to the prototype of the class: + +```javascript +const Foo = Ember.Object.extend({ bar: 'baz' }); +const foo = Foo.create(); + +console.log(Foo.prototype.bar) // 'baz' +foo.hasOwnProperty('bar') // false +``` + +This differs from the behavior of ES Class properties, which initialize their +value on the instance of the class. + +```javascript +class Foo { + bar = 'baz' +} + +const foo = new Foo(); + +console.log(Foo.prototype.bar) // undefined +foo.hasOwnProperty('bar') // true +``` + +The above is essentially currently compiled down by Babel to the following: + +```javascript +class Foo { + constructor() { + this.bar = 'baz'; + } +} +``` + +Property assignments like this are always done at the end of the constructor, +and given the requirement that `super` must always be called before properties +are assigned it is unlikely that this will change as the spec progresses. + +While one might intuitively expect class properties to function the same in +ES Classes as they do with Ember Objects, this difference in behavior means that +class properties will always be assigned after properties passed into `create` +are initialized on the object, and thus will always win: + +```javascript +const Foo = Ember.Object.extend({ testProp: 'default value' }); + +class Bar extends Ember.Object { + testProp = 'default value' +} + +const foo = Foo.create({ testProp: 'new value' }); +const bar = Bar.create({ testProp: 'new value' }); + +console.log(foo.get('testProp')); // 'new value' +console.log(bar.get('testProp')); // 'default value' +``` + +This behavior makes sense when you consider that it is equivalent to assigning +values in `init` rather than on the object when it is defined. Rather than modify +`Ember.Object` to treat class properties as default values, this RFC proposes that +we accept the difference in behavior and utilize the constructor to allow users +to set default values, as in the following example: + +```javascript +class Foo extends Ember.Object { + constructor(props) { + props.testProp = props.testProp || 'default value'; + + super(props); + } +} +``` + +This enforces a public API rather than allowing `create` to override values as +it pleases, and is more inline with the behavior of components in Glimmer today - +args that are passed into the class are distinguished from properties that are +defined on the class. + +## Mixins + +Mixins are a contentious part of both the Ember Object model and the wider +Javascript community - some swear by the pattern, and others believe it fundamentally +flawed. While Ember mixins are at the core of Ember Object, the fact is that +no standard solution for them has arisen in the wider Javascript community as +of yet. + +Additionally, while concepts like computed properties, actions, and +service injection are either unique to Ember or highly dependent on implementation, +mixins can be implemented in a generic way which could be used across all of +Javascript, independent of one's framework or library of choice. With that in +mind, this RFC considers mixins out of scope and suggests that in the future Ember +users can choose to use a mixin library if it suits their needs. + +It should also be noted that existing classes which have used mixins can still be +extended using ES Class syntax: + +```javascript +const Mix = Ember.Mixin.create({ bar: 'baz' }); +const Foo = Ember.Object.extend(Mix, { /* ... */ }); + +class Bar extends Foo { /* ... */ } + +const bar = Bar.create(); + +console.log(bar.get('bar')); // 'baz' +``` + +## Observers and Events + +Observers and events both fail to work properly when using ES Class syntax. The root +of the issue here is how `Ember.Object` works at a fundamental level, and will require +some refactoring to fix. + +Currently, each time `Ember.Object.extend` is used, it stores the list of mixins and +objects passed in on a list which also contains the superclass's properties and mixins, +and so on. A class is then returned which has access to a closure variable, `wasApplied`: + +```javascript +makeCtor = function() { + wasApplied = false; + + return class { + constructor() { + if (!wasApplied) { + this.proto(); + } + } + } +} +``` + +The `proto` function walks the chain of stored mixins, collapsing them into a single object +prototype the first time the class is created. It is during this walk that observers and +events listeners are applied and finalized, as well as merged and concatenated properties +applied (this will be touched on more in the next section). + +Unfortunately, due to the nature of how observers and event listeners work, they cannot be +applied at class definition time without a class decorator. For example: + +```javascript +const Foo = Ember.Object.extend({ + fooObserver: Ember.observer('foo', function() { /* ... */m }) +}); + +class Bar extends Foo { + fooObserver() { /* ... */ } +} +``` + +When `proto` walks the mixin chain for Foo, it will add an observer that triggers the +`fooObserver` function whenever `foo` changes. Bar, however, overloads the `fooObserver` +function with a function that is _not_ observed, and thus should not trigger (this is +analagous to how Ember Object's work today). Currently there is no time at which +Bar can inspect undecorated properties to determine if the superclass has already defined +them and if they are observed and thus should have the observer removed. + +To fix this, the `wasApplied` state should be moved to the ember meta object on the +class itself, so that both Ember Objects and ES Classes can track if they have had it +applied. Additional logic will also need to be added to allow the current "squashing" +behavior of `proto` to work with Prototypes instead of a list of mixins as well. + +## Merged and Concatenated Properties + +Ember Objects currently have the ability to define special properties which are +merged or concatenated with their superclass when extended. This is most commonly +seen with `actions` and `classNames` among others. + +As mentioned in the last section, merged and concatenated properties are also +combined during the `proto` "squash" phase, and so it is also broken in ES Classes +currently. This RFC proposes that their behavior also be fixed as part of the refactors +to Ember.Object. + +# How We Teach This + +The sole purpose of this RFC is to make the behavior of ES Classes within Ember a +public API so that projects like `ember-decorators` can continue to build and experiment +with confidence that the underlying behavior will not change. The Ember Object model +will remain exactly the same as today, and will continue to be the recommended path +for Ember users. Thus, we will not need to add new documentation for the time being. + +# Drawbacks + +* Making `constructor` a public API means we are solidifying the lifecycle of +objects, locking us into a particular sequence of events (`init` occurs within +the `super()` portion of the constructor). +* Lack of mixin support may make it difficult for mixin heavy codebases to utilize +ES Classes. +* ES Class features/usage such as getters and setters may confuse users in general +(getter functions will _appear_ to work, but without a `computed` decorator will +not update, etc.) + +# Alternatives + +* Class property initialization can be changed such that properties are initialized +after the constructor runs entirely, allowing them to be overwritten by values +passed to `create` + +# Topics for Future RFCs + +While working on this RFC, some issues were brought into focus regarding existing +features in CoreObject that are seen as problematic or unintuitive. In order to +avoid bikeshedding these have been slated for discussion in future RFCs, but the +discussion points have been included below. + +## Merged and Concatenated Properties + +Merged and concatenated properties are pain points for new Ember developers, +specifically because they give no lexical hint that they are special in any way. +Developers must know that these particular properties will be merged with the +superclass, and there is no way to opt out of this behavior. + +With decorators, this same behavior can be accomplished in a much clearer and +more straightforward way: + +```javascript +class FooComponent extends Ember.Component { + @concatenated classNameBindings = ['foo'] + + @computed + get foo() { /* ... */ } + + @merged actions = { + bar() { /* ... */ } + } +} +``` + +They could also be accomplished more ergonomically with specialized decorators: + +```javascript +class FooComponent extends Ember.Component { + @className + @computed + get foo() { /* ... */ } + + @action + bar() { /* ... */ } +} +``` + +This approach has two distinct advantages over the existing behavior: + +1. It is less magical. The decorators indicate to new users that the properties are +special in some way, and ultimately they are just plain decorators, which are +compatible with ES Classes as a whole and can be reused anywhere. +2. It provides a way to opt out of the behavior. Currently, there is no easy +way to prevent properties which were marked to be merged from being merged, +meaning subclasses are stuck with the values that their superclass provided. + +## Observers and Listeners + +Observers and event listeners are a powerful pattern that saw a lot of usage in Ember 1. +However, it is now widely accepted that they are problematic when overused, and using +computed properties and lifecycle hooks are better patterns in most cases. + +As such, rather than having events and observers turned on by default it may make +more sense to have them be opt-in APIs. This could be accomplished by making new +class decorators like so: + +```javascript +@evented +class Foo extends Ember.Object { + @on('init') + onInit() { + // do something + } +} +``` + +Or it could be accomplished with new base classes that include the functionality: + +```javascript +class Foo extends EventedObject { + @on('init') + onInit() { + // do something + } +} +``` + +# Unresolved questions + +None currently