Deprecation Shaking

[pzuraq]

Rendered

[jrjohnson]

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

[pzuraq]

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?

[jrjohnson]

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.

[rwjblue]

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).

[jrjohnson]

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.

[runspired]

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.

2. 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"

[NullVoxPopuli]

eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee

[Gaurav0]

Ember Data has implemented this using the traditional options hash in ember-cli-build.js / index.js and using a "compatWith" name. I'm wondering what are the advantages to moving this to package.json?

I feel like we have configuration in too many places already. Furthermore, package.json does not support comments.

[pzuraq]

@Gaurav0 I think it would be much easier to update automatically with tools like ember-cli-update for one thing, and the static nature of it means it's much easier to analyze ahead of time, quickly. It also prevents any kind of dynamic reassignment of the option at build time, or issues caused by incorrectly calling included, etc (though that's just a general problem).

Are there any particular advantages to having an option in ember-cli-build?

[runspired]

I like the shift to package.json given it increases the ability for static-analysis tools to handle this as well as for lower-cost runtime analysis if needed.

As @Gaurav0 mentioned we allow for this compatibility to be configured via the ember-cli-build file currently by specifying the option emberData: { compatWith: '<semver-version>' }

While I think we should absolutely add since and source to deprecate, I'd note that this doesn't on its own opt anyone into any deprecation stripping. I don't see any mention in this RFC of how to configure an addon to have deprecated features removed from it when a user specifies their compliance.

in EmberData we use static booleans with babel-plugin-debug-macros to achieve this whose value is calculated at build time based on the compatWith version.

embroider/macros is trying to solve a similar problem space as babel-plugin-debug-macros.

Is the recommendation that folks roll their own infra for addons? If so, I suspect this RFC design is prone to create confusion in the community when a compatibility version is specified yet the deprecated behavior is not removed. It would be especially confusing to strip the deprecate calls if the deprecated code was not also stripped.

[jrjohnson]

I hadn't thought about that, but I agree with @Gaurav0 that comments on this are critical. See our deprecation-workflow.js for an example. Otherwise it's very difficult to keep track of why something isn't at the most recent version and then take another look at the right things on the next pass.

[Gaurav0]

I should point out that @ember/optional-features does things a third way, with its own optional-features.js file. I think having all this configuration in so many places is confusing and makes things difficult for ember-cli users.

[runspired]

Another question: how do we address deprecations in past releases don't specify since or source?

[rwjblue]

Another question: how do we address deprecations in past releases don't specify since or source?

We build up a cross reference using the id's of the deprecations.

[rwjblue]

I should point out that @ember/optional-features does things a third way, with its own optional-features.js file.

I agree that optional features could easily be merged into package.json's ember key (and provide similar help), we can do that in a subsequent RFC after the dust settles on this one.

[runspired]

We build up a cross reference using the id's of the deprecations.

@rwjblue mind expanding on this a bit for me? I'm curious how this is going to be done. Can be discussed OOB.

[pzuraq]

@runspired

I don't see any mention in this RFC of how to configure an addon to have deprecated features removed from it when a user specifies their compliance.

This was left as an unresolved question in the RFC. As written, the point of the RFC is not to provide tooling for removing deprecated features, just to turn deprecations into assertions at a given point in time.

Providing tooling for actually stripping code presents two possible problems:

1. APIs for users to strip/change code based on other addon's compliance. For instance, an addon could want to stop using a feature in ember-source IFF the parent app/addon no longer uses it. In general, we don't want to encourage this kind of forking of behavior based on deprecation-shaking - addons should always remove deprecated features in all codepaths, as possible.

2. APIs for users to strip their own deprecated features based on compliance of their own addon. This is acceptable, but presents the question of how to do it without allowing 1, and also is a larger lift. It could be added in the future.

Note, for the use cases for 1, we outlined in the RFC how optional features will likely be a better fit for that style of shaking. If Data for instance wants to drop native class support, they could do so if the (future, tbd) Classic class optional feature is disabled.

[runspired]

@pzuraq nice :) Seems like a good time to extract the deprecation assertion test helpers from EmberData for the rest of the world to use in their tests :)

[cibernox]

I can't express how much this excites me. I've optimizing an app as much as possible until I reached ~190kb after gzip, but it's almost impossible to lower that number with Ember itself taking ~150kb. Any effort towards reducing the payload for apps not using deprecated features is a big win.

[xg-wang]

Embroider is proposing macro system where addon can do something like

import { dependencySatisfies, macroCondition } from "@ember/macros";
if (macroCondition(dependencySatisfies("ember-data", "^3.0.0"))) {
  // include code here for ember-data 3.0 compat
}

In terms of tooling for addon authors this RFC could enable Embroider to add another macro for compatibility check.

[pzuraq]

This RFC was superseded by Deprecation Staging, which introduced a different mechanism for specifying compatibility with deprecations. While that RFC does not explicitly discuss "shaking" unused deprecations, the end result is that because deprecated features will assert and not be used, they can also be removed from production builds at will, without a need for a separate system for doing so. There is no need, IMO, to RFC shaking behavior.