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.

Sign up for GitHub

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

Decorator Support RFC #440

Merged
merged 8 commits into from
Mar 15, 2019
Merged

Decorator Support RFC #440

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d147055
Decorator Support RFC
Feb 7, 2019
11096d6
updates based on feedback
Feb 7, 2019
a456769
Add PR link
pzuraq Feb 7, 2019
72a2168
Update text/0000-decorator-support.md
mike-north Feb 8, 2019
bf7c70e
add constraints list
Feb 8, 2019
570f186
update to support stage 1
Mar 2, 2019
3cd45c8
loosen stage 3 constraint
Mar 8, 2019
3ae9a82
Rename 0000-decorator-support.md to 0440-decorator-support.md
pzuraq Mar 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions text/0000-decorator-support.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
- Start Date: 2019-02-06
- Relevant Team(s): Ember.js, Ember Data, Ember CLI, Learning, Steering
- RFC PR: https://github.com/emberjs/rfcs/pull/440
- Tracking: (leave this empty)

# Decorator Support

## Summary

Move forward with the Decorators RFC, and support decorators while they remain
in stage 2 and move forward through the TC39 process.

## Motivation

The recently merged [Decorators
RFC](https://github.com/emberjs/rfcs/blob/master/text/0408-decorators.md)
contained an explicit condition: That it was premised on decorators moving from
stage 2 in the TC39 process to stage 3. If this were to not happen, the original
RFC stipulates that a followup (this RFC) should be created to clarify whether
or not we should move forward with decorators while they remain in stage 2, and
how we would do so.

This RFC proposes that we do so. As discussed in the original, the decorator
pattern has been in use in Ember since the beginning, and there is no easy path
forward to native class syntax without a native equivalent to "classic"
decorators.

[Stage 2 in TC39](https://tc39.github.io/process-document/) signals that the
committee expects this feature to be included in the language in the future, and
is working out the details and semantics. The fact that decorators _remain_ in
stage 2, and have not been rejected, signals that they still expect this to be
the case. This is also the feedback that the core team received in the January
TC39 meeting. _Crucially_, all parties were in agreement about the _invocation_
syntax of decorators:

```js
class Person {
@decorate name;
}
```

This is the most stable part of the spec, and the one that average developers
will come into contact with most. It means that changes to the spec will likely
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the before-export and after-export discussion still underway, or has that settled? This is a part of the invocation syntax that has changed a bit over recent months

Copy link
Contributor Author

@pzuraq pzuraq Feb 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stage 2 settled on decorators after, but this could change since this has been a point of contention. However, no decorators in Ember itself are currently class decorators, and there aren't any plans currently to add them. Decorators in external libraries such ember-decorators will experience some amount of churn, but this should be very easy to codemod so it shouldn't have a huge impact.

affect the _application_ of decorators, which will in turn affect library and
framework maintainers, but **not** end users in most cases. This places the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement relies on the assumption that end users will not have decorators to maintain. Is there anything we can do to lower risk in this area, so that we can have more confidence around insulating application developers from changes in decorator specs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most we can do in this area is ensure that we develop solid libraries of decorators that are compose-able and allow users to define their own macros without actually having to write decorator code.

For instance, earlier versions of ember-decorators did not provide a way to write custom decorator macros. If a user want to do the equivalent of something like:

function join(...keys, separator = ',') {
  return computed(...keys, function() {
    return keys.map(k => this.get(k)).join(separator);
  });
}

They would have had to write a macro. With our current design, both in Ember and ember-decorators, the above will Just Work, meaning that users are insulated from having to know the specifics of decorators' implementation.

I believe in most cases we'll find that very few user apps actually need to write their own decorators. Ember has been using the decorator pattern for years, but we haven't felt the need to make the internal decorator (Descriptor) system public. There have only been two external users of this (private API) system, both addons - ember-concurrency and ember-intl. Coupled with the lack of demand, I think this demonstrates that decorators are primarily a library/addon level feature, not an app level feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can definitely add that it is recommended to wait to write your own decorators until after they have stabilized in our documentation, to drive this point home.

burden of dealing with the spec changes on the Ember.js core team, and addon
authors who wish to adopt decorators while they are in stage 2.

As such, adopting decorators now should present a minimal amount of risk to
pzuraq marked this conversation as resolved.
Show resolved Hide resolved
Ember and its users, and while it is not ideal, it allows us to continue moving
forward and innovating as a framework.

## Detailed design

### Classic Classes

The first thing to note is that Ember.js will continue to work with classic
classes and classic class syntax for the forseeable future, as a way for users
to opt-out if they don't want to take the risk of using native classes. This
includes:

- Classic Classes
- Classic Components
- Mixins
- Computed Properties
- Observers
- Tracked Properties
- Injections
- All existing classes defined with classic class syntax

Notably, `GlimmerComponent` will _not_ support classic class syntax, due to its
constraint of having to support both Ember.js _and_ Glimmer.js, where the
classic class model is not available. However, creating an API compatible
classic class version using Ember's component manager API should be possible if
users want to write Glimmer-like component's with classic syntax.

### Decorator Versioning and Support

Because the decorators spec has changed while remaining in a stage 2, it's no
longer enough to refer to "stage 2" as a version. Since Babel will be providing
the transforms, we will use their versioning system. [This is currently in
development](https://github.com/babel/babel/pull/9360), but looks like it will
be based on the TC39 meeting that the spec was presented at. For instance, the
current transforms available in Babel are referred to as `nov-2018`.

Ember will begin by supporting the _latest_ version of the decorators transform
provided by Babel. Currently this is the `nov-2018` version, but by the time
decorators are released this may change.

The decorators version will be configurable on a _per-app/engine/addon_ basis.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the scenario where ember-myaddon requires one version of decorators, and ember-myapp requires another:

  • are we concerned about potential asset size increases, resulting from allowing each engine/addon to define its own decorator version?
  • as an addon author, how do I decide which decorator version I'd like to support?
    • It seems like by upgrading my addon from decorators v16 to decorators v17, I'd be doing apps on v17 a favor by removing helpers needed to support v16, but introducing new costs for apps using v16.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are concerned about this, yes, and we have been talking about ways to mitigate it. The current strategy we have in mind is to allow users to define or build multiple files for each decorator version implemented, and then have a babel transform rewrite the import paths to the version desired in a given addon. Combined with the ongoing work to revamp the build system for packaging and treeshaking, this would allow us to treeshake out any fully unused versions.

This plan is not outlined here however because it may not be possible, and relies on too many unknowns. In the worst case, we may have to continue shipping multiple versions of the transforms for some amount of time. Given that the transforms do not update often, and we are retaining the ability to drop support for versions after LTS releases, this should not amount to much extra weight of code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decorators version will be configurable on a per-app/engine/addon basis.

If I choose not to explicitly configure, am I opening myself up to some of the potential breakage described below,

if an application updates, it could break their decorators

or significant, unexpected asset size increases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you choose not to configure and are only using Ember's official decorators, you are not opening yourself up to potential breakage. You will always be on the latest version of the transforms, and Ember will always ship that version.

If you use an external decorator libraries, you may be opening yourself up to breakage. It depends on whether or not the library authors have updated their decorators.

Asset sizes may increase, but they will likely not be significant. It's unlikely that any library will need to reimplement all of their decorator code, it's much more likely that they will have to write thin translation layers for each version supported.

This will allow the ecosystem to adopt decorators without fears that if an
application updates, it could break their decorators. If no version is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will allow the ecosystem to adopt decorators without fears that if an
application updates, it could break their decorators.

If decorators are included in the framework, I'd expect them to play by the same SemVer rules around breaking changes as everything else. Are we defining this as a non-major-release change? Seems like there may be some qualifications here -- if so can we call them out explicitly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decorators included in the framework will play by the same SemVer rules, which is why allowing the default to "float" forward is fine. Decorators that are provided by external libraries may have different constraints, which is why you can explicitly lock to a specific version of the decorators transform.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this say:

This will allow the ecosystem to adopt decorators without fears that if an
addon updates, it could break their decorators

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is referring to applications updating their version of Ember, and the transforms for decorators along with it. Addons locking their version of the transforms is a preventative measure against this.

specified, the transform will default to the latest version supported by Ember
at that time. In many cases the only decorators in use will be Ember's, so this
allows the ecosystem to upgrade without additional maintenance costs. The exact
method for specifying the version will be determined during implementation, but
will likely be a configuration option or addon.

When new transforms are released, Ember will continue to support older versions
of the transforms alongside the new ones until _at least_ the next LTS release,
with deprecation warnings for any users of the older transforms. This will mean
shipping multiple different versions of decorators at once, and providing a way
for apps/addons to import the correct version. Additionally, Ember will attempt
to provide addon authors with tools that allow _them_ to do this as well, so
they can support multiple versions of transforms easily and transparently. The
exact API for these tools is out of scope for this RFC, and will likely be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exact API for these tools is out of scope for this RFC

Shouldn't a proposed implementation strategy or at least a set of user stories (not specific to any specific decorator spec version) for these tools be included in this RFC?

i.e.,

  • The asset size costs for an addon should not scale with the number of decorator versions it supports

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we can make that kind of guarantee at this point, which is why this RFC is defining the semantics of what we want to support and not focusing on the specifics of the implementation. It may be that supporting multiple versions of decorators does require larger asset sizes, and it will be the responsibility of each addon/app author to weigh the pros and cons of adding more library code to their payloads.

We could maybe outline user stories with the general goals of what we want to achieve, with a caveat that this will be a best-effort goal and not a hard requirement.

dependent on their implementation.

## How we teach this

As mentioned in the motivation section, the _invocation_ side of decorators
seems to be much more stable currently, so we ideally will not have to teach end
consumers too much about the details of this system. We should definitely
provide a disclaimer that outlines how our support structure works, and how
users can lock their version if they are using custom decorators. This should be
included _early_ in the guides.

We should also provide thorough documentation for any tooling provided to help
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specific things I can think of that are probably worth teaching:

  • Do addon CI matricies need to expand to D (decorator version) ⨉ F (framework version) now?
  • If an app pins their decorator version to maintain compatibility w/ an addon, how will they know when it's a good idea to un-pin? If we don't automate this in some way, it would be a good idea to teach it

addon authors write their own decorators. This will be a bit more of a moving
target, since this RFC does not define exact APIs and they will not be a
priority until Ember itself prepares to support multiple versions of the
transforms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this RFC does not define exact APIs and they will not be a priority until Ember itself prepares to support multiple versions of the transforms

Will there be an additional RFC for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends on a few things:

  1. The complexity of the APIs
  2. The number of users in the ecosystem who will have to adopt them

If there are relatively few addons who end up writing their own decorators, and the APIs are relatively simple and folks are aligned on the solution, the RFC process may not make sense here, since the APIs would be temporary.


## Drawbacks

The fact that decorators did not move to stage 3 signals that there may be
additional changes to the spec in the near future. Adopting them now could cause
more churn than it's worth.

## Alternatives

- We could continue to rely on unofficial addons such as `ember-decorators`,
which allow users to opt-in to using decorators. However, these libaries have
limitations that first class decorators will not have, and they don't allow us
to update the official guides to use them.

- We could create an official decorators addon. However, this means that
decorators would be available at a different import path, meaning that any
code which seeks to work with _both_ classic classes and native classes would
have to be written twice. This would be very difficult for applications that
are mid-transition, and even more difficult throughout the ecosystem for addon
authors.