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.

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

Deprecation Shaking #565

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
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
190 changes: 190 additions & 0 deletions text/0565-deprecation-shaking.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
- Start Date: 2019-12-17
- Relevant Team(s): Ember.js, Ember Data, Ember-CLI
- RFC PR: https://github.com/emberjs/rfcs/pull/565
- Tracking: (leave this empty)

# Deprecation Shaking

## Summary

Allow applications and addons to eagerly opt-in to "shaking" deprecated
features, removing them from the builds of apps that do not use them.

## Motivation

Ember has always had strong guarantees around semver compliance and a solid,
stable upgrade path for every major change it makes. This has, over time, led
to a large amount of extra code that exists in the framework - code that is
marked for removal in the future, but can't be removed immediately due to those
guarantees.

This is especially true now that with Octane, which includes a large new set of
features that were ultimately designed to completely replace lots of older
features. Classic features aren't deprecated yet, but at some point in the
future they likely will be, and being able to take advantage of those size
reductions as they come would be a huge selling point for Ember apps.

## Detailed design

This RFC proposes allowing users to specify deprecation compliance for both apps
and addons via a field in the `ember` property in `package.json`:

```json
{
"ember": {
"edition": "octane",
"deprecationCompliant": {
"ember-source": "3.15.0"
}
}
}
```

Deprecation compliance can be specified for any Ember addon, including
`ember-source`. Specifying compliance for a particular version of an addon means
that the consuming app/addon will function without triggering any deprecations
that were added in or prior to that version. Specifying the version number will
turn all deprecations before the version into _assertions_ when running in DEBUG
mode, preventing users from relying on the behavior altogether, but still
providing a helpful error message in those cases. In production builds of
`ember-source`, the features will be removed as possible, but they are not
guaranteed to be removed fully.

The `deprecationCompliant` field must be an exact, existing version of the
addon. New applications and addons will have `deprecationCompliant` set to the
latest version of `ember-source` and `ember-data` once the feature has landed.

Ember's `deprecate` method will receive two new arguments that allow it to
participate in this system:

```ts
interface DeprecationOptions {
id: string;
until: string;
since: string;
source: string;
url?: string;
}

declare function deprecate(message: string, test?: boolean, options?: DeprecationOptions) => void;
```

`since` must be a SemVer version equal to or lower than the current version of
the library. `source` must be the name of the library that is creating the
deprecation. If either of these options is missing, then the deprecation will
not be affected by any settings for deprecation compliance in consuming
applications. Not including these options will be deprecated and issue a
warning.

### Dependencies

Apps and addons may depend on other addons that are not yet deprecation
compliant with a given version. If the parent app or addon has a more recent
version of a library specified, an error will be thrown, alerting the user of
the addon that is not compliant. The user can then lower their deprecation
compliance version to match the addon if they choose, but they _cannot_ override
the compliance versions of their dependencies, since there is no way for us to
know whether or not the feature is used and will cause errors otherwise.

If an addon does not specify a deprecation compliance version, then it will be

Choose a reason for hiding this comment

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

I don't see how this will work. Addons which specify nothing will get deprecated features stripped, but addons which specify something will block apps which have a newer version. This places a difficult burden on addon maintainers either:

  1. continuously update this value to a new version even when no changes need to be made to the code itself.
  2. leave it out and stay rigidly on top of deprecation warnings to avoid breaking apps who specifiy a new ember version

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 called out in the drawbacks as a downside. I think automatically updating the compat version with ember-cli-update would help addon authors, but I'm not sure there's another way we could provide the support we're trying to, without these caveats.

Allowing more recent versions/overrides from the consumer would just mean that the versions addon's specify has no safety at all, and the other option would be that addon's don't specify a version and we don't warn the user at all.

Any ideas?

Choose a reason for hiding this comment

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

I don't think addons need safety here. Application authors get deprecation warnings so they should be responsible for not setting this version too high for the addons they consume. I don't think this value should be set anywhere but the top level app.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think addons need safety here.

I think I disagree, though only slightly. I think there are absolutely a set of addons that will want to explicitly declare their compatibility, but that most will want to leave this unspecified in their published package.json. Instead, addons would ensure that they do not regress on the deprecation front by specifying ember.deprecationCompliant.ember-source in their ember-try configuration. This ensures that the addon author is aware of the deprecations being emitted from the addon itself (during their normal workflow), and that consumers are not negatively affected (assuming the consumer app/addon's own tests pass properly).

Copy link

@jrjohnson jrjohnson Dec 18, 2019

Choose a reason for hiding this comment

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

Then can we lean towards this RFC and resulting documentation recommend addons not specify this information unless they want an unhanded deprecation in their code to throw a build error in apps? Is there another case I'm not seeing?

Without that I'm seeing a future where the addon blueprint declares the current version, but as time goes by that version declaration (not any specific deprecation) causes failures and has to be updated, but updating it is scary, what if you break something, so a simple PR to update it goes un-merged. I remember how hard the babel 5 move was to get all addons updated and I'm imagining having to do this every 6 weeks for every ember release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have two thoughts on addons:

  1. Can we use a compound key? the addon cannot declare compatibility for a version greater than the version of ember-source it declares in dev.

This means that addons that specify nothing wouldn't drift accidentally into non-compat over time.

  1. Can we add an addon-overrides to the app's config? TL;DR the app should be able to forcibly say "x addon is compatible"

assumed to be compliant.

### A Note on Per-Feature Shaking and Optional Features

An [earlier version of this feature](https://github.com/emberjs/rfcs/issues/532)
proposed allowing users to specify which features they wanted to disable
explicitly. The thinking was that this would prevent deprecations from being
blocked on each other, and enable more aggressive deprecation timelines in
general.

However, attempting to do this in a generic way would have resulted in a
combinatorial explosion of possible deprecations that could interact with one
another. If a user chose, for instance, to enable shaking for observers, but not
for computed properties, we may not actually be able to know if we could safely
shake all of the related code, since they share a lot of the same
infrastructure. It would also be very difficult to _test_ all possible
combinations of enabled/disabled deprecations. Shaking all deprecations prior to
a minimum version gives us one target combination, which is much easier to
reason about internally, and much easier to test.

Instead, we can use Ember's existing **optional features** infrastructure to
allow users to disable larger, more embedded features within Ember eagerly.
Optional features are generally reserved for larger shifts in Ember's behavior,
so there are fewer combinations to test compared to deprecations, which are used
for large and small things.

Optional features also send a better signal to the community. Major features
like Classic classes, observers, and computed properties can first go through a
period of being optional, before eventually being deprecated, and then removed.
Transition to an optional feature is a signal that the feature will eventually
be removed, but not in the near future, so it's not an
upgrade-now-or-forever-be-stuck-on-the-last-major-version-of-Ember level
priority. The feature can be converted to a true deprecation when the community
is ready and most apps and addons have converted to new idioms.

## How we teach this

This should be included in a guides section on performance and build size. It's
an advanced topic, so it should not be placed in a section aimed toward
beginners.

### API Docs

#### `deprecationCompliant`

This option specifies the versions of libraries that the application or addon is
_deprecation compliant_ with. Deprecation compliance means that the app/addon
does not use any features that have been deprecated on-or-before that version,
and does not trigger any warning messages when run.

```json
{
"ember": {
"edition": "octane",
"deprecationCompliant": {
"ember-source": "3.15.0"
}
}
}
```

Telling Ember the version that the app/addon is compliant with will cause all
deprecations for that library prior to the specified version to throw errors
instead of warnings in development builds. Libraries may also attempt to remove
that code whenever possible in production builds, slimming down the final build
and app size.

`deprecationCompliant` should be set to an object whose keys are the names of
packages that the app/addon depends on, and whose values are the exact SemVer
version that the app/addon is compliant with.

##### Dependencies

The deprecation compliant version of an app/addon cannot be greater than any of
their dependencies compliant versions. Ember CLI will throw an error if it finds
that one of your dependencies has an older version, and will let you know what
version you can lower your own compliance to.

If a child addon does not specify compliance for a given library, it is assumed
to be compliant, and will not cause errors at build time. The library may still
not be compliant however, and could cause assertions to be thrown, so you should
be aware of this and report the issue to the library directly if this occurs.

## Drawbacks

- This increases complexity for addon authors, who will likely receive pressure
to update their compliance more frequently from apps that make use of these
features.

## Alternatives

- A "flag" version of deprecation shaking, where users decide which features to
disable and attempt to shake. This has several problems, which are covered
above in the Detailed Design section.

## Unresolved questions

- Should `ember-cli-update` attempt to automatically update the compliant
version?
- Should Ember provide any utilities for checking and shaking deprecated
features at build time for other addons?