Specifying and detecting an applications edition.

[rwjblue]

Rendered

Motivation

As Ember approaches it's first edition (see emberjs/rfcs#364) various addons need to modify their behavior based on the edition that is being used. An initial implementation (done without RFC) used the setEdition method from @ember/edition-utils inside the application's (or addon's) .ember-cli.js file to specify which edition was to be used. That implementation worked well enough throughout the initial preview period, but a number of major issues were (rightfully!) surfaced by the community:

1. it seems unnecessary/redundant
2. it's not clear what this flag actually does (likely due to having no RFC!)
3. it's not statically analyzable (and therefore cannot be used by things like codemods)

This RFC will review these issues in light of the updated implementation, showing how each of the concerns have been met.

Timeline

This RFC will have a significantly compressed timeline (hopefully in FCP tomorrow 🙀), as we attempt to move forward with "shipping octane" (🤞in a version very soon). I am sorry that it has taken me so long to put together, but hopefully we can come to a quick consensus and fix the mistakes of the past.

[NullVoxPopuli]

💯 I think the ember root key in package.json gives us room to play around in the future, too.

[Turbo87]

LGTM, thanks again for working on this 👍

[jrjohnson]

Couple of add-on author questions. Is this also where addons declare their support? The edition for the dummy app? How do addons test multiple editions?

[runspired]

[chriskrycho]

I like it a lot better! Had a couple typo corrections, but

[rwjblue]

Couple of add-on author questions. Is this also where addons declare their support? The edition for the dummy app? How do addons test multiple editions?

@jrjohnson - Great questions! I have added a snippet to the RFC to specifically address this. Here is the relevant verbiage added:

For applications specifying ember: { edition: 'octane' } in package.json is
generally all that is needed. In an addon this new property would be specifying
the edition that the addons own dummy app uses. However, many addons may want
to test against multiple editions that they support. In order to support
this, ember-try will be updated to allow specifying (and merging) the ember
property in the package.json from their config/ember-try.js scenarios.

[rwjblue]

What about changing the build? For instance loading different class implementations.

@buschtoens - Great question, I've updated the prose to explain what the best path forward is here. Snippet below:

Note that the above definition does not allow for an addon to detect the
edition configuration and change its fundamental implementation. This is quite
intentional!

Note that the above definition does not allow for an addon to detect the
edition configuration and change its fundamental implementation. This is quite
intentional!

Instead, addons should rely on feature detection techniques like the following
to alter implementations:

• Check to see if given methods/properties exist (this is essentially like how browser polyfills work)
• Use a tool like ember-compatibility-helpers's gte method (e.g. if (gte('ember-source', '3.14.0')) { })
• Use @ember/optional-features's isFeatureEnabled method

[kellyselden]

Can you clarify why .ember-cli won't work? Is it because .ember-cli only looks like JSON, but is interpreted, so who knows what could be in there?

Or is it because we also support .ember-cli.js, it would force them to have both files to get the static edition?

[rwjblue]

Can you clarify why .ember-cli won't work?

@kellyselden - Ya, I had this question specifically listed in the "alternatives" section, but I've updated the prose there to hopefully make it even clearer. Please let me know if that helps!

Snippet:

Instead of adding the emberEdition value to the package.json we could add it to the existing .ember-cli file. However, doing this would not satisfy the static analysis constraint mentioned in the motivation section (because .ember-cli.js is transparently supported by ember-cli's build system). In addition, any values that are included in .ember-cli are automatically passed in to every command invocation which would be both unintended (we don't want commands to access the edition in this way) and possibly breaking (if the command already accepted an option with whatever value we chose).

[kellyselden]

and possibly breaking (if the command already accepted an option with whatever value we chose).

Isn't that also true of any apps that use an "ember" property in package.json for whatever reason?

The snippet doesn't really explain the relationship between .ember-cli.js and .ember-cli. I know it's not a requirement of the RFC to explain it, but it might be nice for readers to explain why supporting a file called .ember-cli.js means we can't use static JSON in .ember-cli for this new property. Thanks.

[rwjblue]

Thank you all for your help in moving this forward! We discussed this in the Ember core team meeting today and are moving this into final comment period.

In addition, due to the shortened cycle for this RFC (apologies again 😞 ), we will be working toward landing implementations in ember-cli's current beta cycle (targeting 3.15's release in a few days). Regardless of landing the implementation in the beta branch, any actual release will wait for this RFC cycle to be completed (assuming no new information / issues are brought up in the thread here, this means a release could happen as early as next Friday). I realize this is a somewhat unusual (we normally like to have a full cycle for new features), but hope that y'all understand.

[rwjblue]

The snippet doesn't really explain the relationship between .ember-cli.js and .ember-cli. I know it's not a requirement of the RFC to explain it, but it might be nice for readers to explain why supporting a file called .ember-cli.js means we can't use static JSON in .ember-cli for this new property.

@kellyselden - Ya, I'll add a bit more detail there. I agree the current "oh, btw, there is also .ember-cli.js" doesn't actually explain why that is a deal breaker.

[rwjblue]

The core team met to discuss this RFC today, we are still in favor of moving forward here, there were no new concerns brought up during the FCP period, the implementation has gone smoothly, we are ready to land this.

Thank you everyone for providing feedback, we absolutely ended up on a much better solution!