Make addons honour app's transpilation by default #89

cibernox · 2016-12-07T20:05:34Z

…ripping Ember CLI: Production code stripping

Turbo87 · 2016-12-07T20:10:42Z

active/0000-control-transpilation.md

+some experimental ones.
+
+What is less know is that this configuration only affects the application code. The transpilation
+of code living in addons is done according to the configuration of those addons. While there is


to be specific: the app config affects the transpilation of everything inside app/, even those app/ folders inside of addons!

Turbo87 · 2016-12-07T20:12:14Z

active/0000-control-transpilation.md

+# Detailed design
+
+The idea is to still allow addons to be in control of their own transpilation process, but
+make them honour uses preferenced by default.


typo: user preferences

Turbo87 · 2016-12-07T20:12:30Z

active/0000-control-transpilation.md

+
+# Motivation
+
+Ember-cli as allowed users to write ES6 code and transpile it to ES5/3 since almost the beginning


either ember-cli or Ember CLI

also as -> has

Turbo87 · 2016-12-07T20:14:13Z

active/0000-control-transpilation.md

+
+By example, an addon that uses `async-await` which right now are only available in Chrome Canary
+could either use the host app's configuration plus this feature or it could just assume the user
+has enabled it and tell them to do so in the docs.


or it could use a blueprint that adjusts the Babel settings for the user

I prefer this approach, I think. The addon shouldn't require anything here, it should possibly specify which features it needs support for, and then get venn diagram processing.

Maybe it's cooperative? The root application specifies what its target is, the child addons specify how to achieve that goal?

Turbo87 · 2016-12-07T20:17:46Z

active/0000-control-transpilation.md

+
+None that I can think of, beside of not doing this.
+
+# Unresolved questions


we would also have to figure out how Babel resolves plugins. normally they are referenced by name and loaded relative to the Babel package itself, which can be a problem when e.g. ember-cli-babel is npm linked.

Apparently plugins can also be just required and passed to Babel.

that is somewhat correct, yes. this does not work inside of a .babelrc file though which is JSON-based.

I believe that the app should parse its .babelrc file and convert it to a JS object. This object is the one that gets deepcloned and passed to addons so they can manipulate it.

I think that for addons that need to mangle the transpilation the .babelrc manifest is not expresive enough.

Please note: this RFC does not suggest migration to .babelrc

@rwjblue I know, but I'd like us to be aware of that option. We shouldn't build anything that blocks our way of using those files if we want to.

@Turbo87 - Ya, I completely agree. I wasn't very clear either, I think the RFC should address (or at least mention) .babelrc and other concepts that we are required to chart through on our road to babel@6 (see other comment in #89 (comment)).

Just as reference there is a PR to add a .js to babelrc babel/babel#4892

create-react-app and others do a require.resolve: https://github.com/facebookincubator/create-react-app/blob/ab3913cc113b2392fa0329f699571bae3c213308/packages/babel-preset-react-app/index.js#L15

rwjblue

I really like this, well done!

rwjblue · 2016-12-07T20:10:50Z

active/0000-control-transpilation.md

+Today that is done through `ember-cli-babel`, an addon that is in the default blueprint and
+that the huge majority of apps use.
+
+This transpilation process has some sensitive defaults that work for most people, but also allow


s/sensitive/sensible/ (I think)

rwjblue · 2016-12-07T20:11:03Z

active/0000-control-transpilation.md

+
+# Motivation
+
+Ember-cli as allowed users to write ES6 code and transpile it to ES5/3 since almost the beginning


ember-cli has

rwjblue · 2016-12-07T20:11:28Z

active/0000-control-transpilation.md

+some use configuration for, by example, disable some transformations enabled by default or enable
+some experimental ones.
+
+What is less know is that this configuration only affects the application code. The transpilation


s/know/known/

rwjblue · 2016-12-07T20:12:31Z

active/0000-control-transpilation.md

+
+# Detailed design
+
+The idea is to still allow addons to be in control of their own transpilation process, but


Slight tweak suggested here:

The idea is to still have addons control their own transpilation process, but

rwjblue · 2016-12-07T20:12:45Z

active/0000-control-transpilation.md

+The idea is to still allow addons to be in control of their own transpilation process, but
+make them honour uses preferenced by default.
+
+There is three different scenarios for addons:


rwjblue · 2016-12-07T20:17:03Z

active/0000-control-transpilation.md

+
+This has no impact for apps, just for addons.
+
+Most addons will not require any change. Only those that want to modify this the transpilation


I do not actually see how ember-computed-decorators needs to be changed at all to support this. If you read through the recommended snippet here:

init: function(app) { this._super.init && this._super.init.apply(this, arguments); this.options = this.options || {}; this.options.babel = this.options.babel || {}; this.options.babel.optional = this.options.babel.optional || []; if (this.options.babel.optional.indexOf('es7.decorators') === -1) { this.options.babel.optional.push('es7.decorators'); } }

You will notice that it already honors any this.options.babel settings that are present.

rwjblue · 2016-12-07T20:18:35Z

active/0000-control-transpilation.md

+
+#### How can addons be in charge of their own transpilation and yet honor user preferences?
+
+Ember-cli should provide to each addon a deep clone of the configuration defined in the host app


We must address how to do this properly with the upcoming babel@5 -> babel@6 transition. It is possible that this RFC would be blocked on that transition being completed, or only take effect when both app and individual addons are using babel@6.

This needs more thought/design, but I think it is doable...

I think that the idea of deep-cloning the host's app and use that is universal, whatever version of babel we use.

There is an obvious problem if the app uses babel6 and the addon expects babel5, because the options are not compatible between them, but seems a problem of the babel upgrade more than a problem of this RFC per se.

I think that that want to customize the transpilation will have to support both babels at least for a while.

There is an obvious problem if the app uses babel6 and the addon expects babel5, because the options are not compatible between them, but seems a problem of the babel upgrade more than a problem of this RFC per se.

@cibernox - Right, I totally agree with your assessment here. However, I am willing to completely block this RFC so that we can focus on the babel@6 upgrade if this will make that migration more complicated. If we want to ship this sooner, we need to at least address how it doesn't make matters worse (via some sort of mitigation as I suggested above).

I would prefer to get to Babel6 first and then land this RFC regardless of any design that demonstrates painlessness. I simply think it's a cleaner migration path less likely to leave skeletons behind.

Turbo87 · 2016-12-07T20:58:26Z

one thing to keep in mind when thinking about this RFC is https://github.com/babel/babel-preset-env, which I would like to eventually adopt as the default for Babel transpilation in Ember CLI.

nathanhammond

Finishing this review since it got updated out from under me.

nathanhammond · 2016-12-07T21:32:58Z

active/0000-control-transpilation.md

+
+# Motivation
+
+Ember-cli as allowed users to write ES6 code and transpile it to ES5/3 since almost the beginning


nathanhammond · 2016-12-07T21:33:17Z

active/0000-control-transpilation.md

+of the project. It's one of those things that people take for granted.
+
+Today that is done through `ember-cli-babel`, an addon that is in the default blueprint and
+that the huge majority of apps use.


apps and addons

nathanhammond · 2016-12-07T21:33:39Z

active/0000-control-transpilation.md

+Today that is done through `ember-cli-babel`, an addon that is in the default blueprint and
+that the huge majority of apps use.
+
+This transpilation process has some sensitive defaults that work for most people, but also allow


s/sensitive/sensible

nathanhammond · 2016-12-07T21:34:36Z

active/0000-control-transpilation.md

+that the huge majority of apps use.
+
+This transpilation process has some sensitive defaults that work for most people, but also allow
+some use configuration for, by example, disable some transformations enabled by default or enable


- , by example, disable + disabling - enable + enabling

nathanhammond · 2016-12-07T21:35:25Z

active/0000-control-transpilation.md

+some use configuration for, by example, disable some transformations enabled by default or enable
+some experimental ones.
+
+What is less know is that this configuration only affects the application code. The transpilation


s/know/known/

nathanhammond · 2016-12-07T21:35:43Z

active/0000-control-transpilation.md

+some experimental ones.
+
+What is less know is that this configuration only affects the application code. The transpilation
+of code living in addons is done according to the configuration of those addons. While there is


s/there is/there are

nathanhammond · 2016-12-07T21:37:30Z

active/0000-control-transpilation.md

+#### The addon doesn't care about the transpilation process.
+This is the most common common situation. Most addons are authored in ES6 and they don't
+mess with the host app's configuration.
+Those addons should just transpile their code user the host's apps settings.


s/user/using/, s/host/root/

nathanhammond · 2016-12-07T21:40:27Z

active/0000-control-transpilation.md

+
+By example, an addon that uses `async-await` which right now are only available in Chrome Canary
+could either use the host app's configuration plus this feature or it could just assume the user
+has enabled it and tell them to do so in the docs.


I prefer this approach, I think. The addon shouldn't require anything here, it should possibly specify which features it needs support for, and then get venn diagram processing.

Maybe it's cooperative? The root application specifies what its target is, the child addons specify how to achieve that goal?

nathanhammond · 2016-12-07T21:42:04Z

active/0000-control-transpilation.md

+
+#### How can addons be in charge of their own transpilation and yet honor user preferences?
+
+Ember-cli should provide to each addon a deep clone of the configuration defined in the host app


I would prefer to get to Babel6 first and then land this RFC regardless of any design that demonstrates painlessness. I simply think it's a cleaner migration path less likely to leave skeletons behind.

nathanhammond · 2016-12-07T21:45:11Z

active/0000-control-transpilation.md

+custom plugins, enable specific features or just throw a warning message if the app's config
+doesn't have required specific transformation enabled.
+
+By modifying this clone, their changes will not affect the host app or other addons.


It's theoretically possible that the config defines functions that have scopes and side effects such that the clone isn't valid. We could use vue's global proxy pattern: http://www.2ality.com/2016/11/trace-globals-proxy.html

nathanhammond · 2016-12-07T21:46:53Z

active/0000-control-transpilation.md

+For this approach it doesn't matter if the version of babel is 5 or 6, however the plugins and their
+names are funamentally incompatible.
+
+- Should addons that depend on `ember-cli-babel` be compiled with their version or with the app's version?


I'd prefer we only allow this behavior with homogenous versions. If you're not all Babel6 you get current behavior.

Im not sure.

Allowing the system to progressively transition has its benefits. For example, as add-ons upgrade more and more of the users code-gen would no longer need to cater to the lowest common denominator (ES3).

The downside to this, is you may have some ES3 safe code, mixed with your ES6 safe code. Which seems ok.

This would allow our users to see incremental progress. Especially if the most popular / largest add-ons (like ember-data or even ember) made the change more quickly.

The alternative, requiring homogenous setup. Requires the entire universe to transition before progress is seen.

I suspect coupling incremental + a Quest issue, would enable the best outcomes. (e.g. incremental rewards for our community)

nathanhammond · 2016-12-07T21:53:58Z

I think I want this to be cooperative. The addon knows which features it is using, the root application knows what its target is (ES3, ES6, whatever). If the transpilation target is understood then the addon should know what it needs to do to get there, starting by building on top of the root application's configuration settings. Which plugins it uses to get there do not communicate exactly what the target is, it specifies only a blacklist (and is thus incomplete).

Moving to this model fits very well with our Broccoli build model where addons are able to build themselves based upon a known target. If we're trying to make the addon guess what to do it will likely always opt for ES3 for max compatibility (I would).

nathanhammond · 2016-12-07T22:08:28Z

This concern is partially alleviated by adopting something like https://github.com/babel/babel-preset-env which @Turbo87 linked us to. If the root app specifies this then the addon knows "what" instead of "how." "How" is simply not enough information to do the right thing 100% of the time.

We should probably encourage adoption of this in root level apps.

/cc @hzoo

cibernox · 2016-12-07T23:45:38Z

@nathanhammond The idea of the app declaring intentions and the addon reacting to those intentions based on the knowledge of the features it uses seems hard.

The app will express is intentions in targets (p.e. lastest 2 versions of current active browsers), but addons might express their needs in features (p.e. async/await).

I'm not sure how to make them understand each other in a way that it's not madness.

Separate the grain from the hay

stefanpenner

This is a good direction for us to take.

Question:

Do we need to concern ourselves with "what happens" when an APP and and Addon have incompatible compilation preferences?

stefanpenner · 2016-12-12T15:20:23Z

active/0000-control-transpilation.md


-By example, applications that only target evergreen browsers may want to disable transpilation of
-all ES6 feature since moden browsers already ship them.
+With ES6 being fully implemented in all modern browsers and many ES2016/ES2017 already starting to


Yup, for this reason. This is quite important to land.

cibernox · 2016-12-12T15:39:02Z

Do we need to concern ourselves with "what happens" when an APP and and Addon have incompatible compilation preferences?

I'd say we don't. The only incompatibility I can think about is an addon enforcing the transpilation of something the user doesn't want to transpile. I think the addon config always win, although I would consider that addon a bad boy for not obeying its parent. But it's not worse than what we have already.

If we are talking about the app providing babel6 config and the addon expecting babel5 config, that is a harder question, but I think that the answer belongs more to the Babel5 -> 6 upgrade rather than this RFC.

Can some addons be transpiled by babel5 and other by babel6? /cc @Turbo87

Turbo87 · 2016-12-12T16:59:33Z

Can some addons be transpiled by babel5 and other by babel6?

That was the original plan and it would be very beneficial in the transition period. Otherwise you end up in a situation where some addons only work with older Babel and some only with Babel 6 and you'd have to choose between them...

stefanpenner · 2016-12-12T17:24:43Z

@cibernox and myself had a quick call:

Summary (from @cibernox's #ember-cli-dev summary of the call):

[8:41 AM]  
the outcome of the hangout is that giving too much power to users for very little benefit can make transitions harder so the idea would be:
1. Make `babel-preset-env` be part of the babel6 upgrade
2. Just provide addons with the `preset` configuration of the parent app, which typically consist on only a list of target browsers, expressed as `chrome latest` or `> 2%`

[8:42]  
I agree on that, I need to check if this still allows users to plug their own transformations, like ember-data does (edited)

[8:43]  
if so, the RFC could pivot towards "Make `babel-preset-env` a default and make addons obey that by default"

hzoo · 2016-12-12T20:10:58Z

babel-preset-env only covers preset-latest (no experimental plugins) so it would be the best default behavior but yeah output depends on browser support

cibernox · 2016-12-12T20:22:32Z

@hzoo now that you're here, one of the open point to discussion of this RFC is how addons get access to the babeljs configuration. Addons must be able to customize how they transpile, by example to add their own plugins that perform things like remove intrumentation in production builds and things like that or enable experimental ones if they use stuff that is not yet in the specs, enable loose mode for perf reasons and such.

At the same time, we want to give to the addon the least amount of configuration possible to do their job. @stefanpenner suggested that they should get only the configuration for env.

By example, if the configuration in a project is

{
  "presets": [
    ["env", {
      "targets": { "chrome": 52 },
      "modules": false,
      "loose": true
    }]
  ]
}

The addon would only have access to:

{
  "targets": { "chrome": 52 },
  "modules": false,
  "loose": true
}

Users can add their own custom transformations having access to just this bit of the configuration?

hzoo · 2016-12-12T20:25:25Z

preset-env itself doesn't include custom transforms so you would have to allow them to append to the preset/plugins array at the top level of the config then? preset-env would only be able to whitelist/blacklist plugins already included if we add those options.

cibernox · 2016-12-12T20:30:59Z

So, the configuration of a project with custom plugins looks like this:

{
  "presets": [
    ["env", {
      "targets": { "chrome": 52 },
      "modules": false,
      "loose": true
    }]
  ],
  "plugins": ["plugin1", "plugin2", require('./lib/my-plugin')]
}

@stefanpenner that makes me think that we might need to pass the configuration pristine to the addons, or alternatively provide a hook/object that gives the user access to only the env entry and the plugins array.

cibernox · 2017-03-13T21:40:39Z

@rwjblue After ember-cli-babel exists beta, this RFC is essentially implemented, right?

rwjblue · 2017-03-13T22:49:40Z

yes, but not the specific API's suggested IIRC

rwjblue · 2017-04-19T15:02:17Z

@cibernox - I think we can close this one, can you confirm (or update to reflect the current state of things)?

cibernox · 2017-04-19T15:10:37Z

@rwjblue Can you confirm that addons that use babel6 inside babel6 applications with targets also use those targets? That is the only bit I'm not sure if it's implemented.

cibernox · 2017-04-21T14:43:43Z

As of ember-cli-babel@6.0.0, this RFC has been implemented. In a few days will be released in the stable channel.

nathanhammond and others added 6 commits December 1, 2016 23:25

Merge pull request ember-cli#50 from GavinJoyce/gj/production-code-st…

e896286

…ripping Ember CLI: Production code stripping

Control transpilation

23509d5

Rething RFC

cb5a605

More updates

34af168

Last changes

cb4bc9b

Add date

ee22727

Turbo87 reviewed Dec 7, 2016

View reviewed changes

rwjblue reviewed Dec 7, 2016

View reviewed changes

Fix many typos

77e2f3f

nathanhammond reviewed Dec 7, 2016

View reviewed changes

cibernox mentioned this pull request Dec 8, 2016

Fixups for recent two pass babel changes. ember-cli/ember-cli#6538

Merged

A lot of changes to make clear the focus of the RFC

d42c79a

Separate the grain from the hay

cibernox changed the title ~~Allow apps to control addon's transpilation~~ Make addons honour app's transpilation by default Dec 12, 2016

stefanpenner reviewed Dec 12, 2016

View reviewed changes

cibernox added 2 commits December 12, 2016 15:26

Update 0000-control-transpilation.md

891f7ec

Update 0000-control-transpilation.md

74a606c

nathanhammond force-pushed the master branch from 4200e6f to 21c205e Compare January 6, 2017 17:58

cibernox mentioned this pull request Jan 10, 2017

Standarize targets #95

Merged

kimroen mentioned this pull request Feb 6, 2017

Addons created in coffeescript can only be consumed by applications with coffeescript kimroen/ember-cli-coffeescript#131

Closed

cibernox closed this Apr 21, 2017


		# Motivation

		Ember-cli as allowed users to write ES6 code and transpile it to ES5/3 since almost the beginning


		None that I can think of, beside of not doing this.

		# Unresolved questions


		# Detailed design

		The idea is to still allow addons to be in control of their own transpilation process, but


		This has no impact for apps, just for addons.

		Most addons will not require any change. Only those that want to modify this the transpilation


		#### How can addons be in charge of their own transpilation and yet honor user preferences?

		Ember-cli should provide to each addon a deep clone of the configuration defined in the host app

Make addons honour app's transpilation by default #89

Make addons honour app's transpilation by default #89

Conversation

cibernox commented Dec 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwjblue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Turbo87 commented Dec 7, 2016

nathanhammond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanpenner Dec 12, 2016 • edited Loading

Choose a reason for hiding this comment

nathanhammond commented Dec 7, 2016

nathanhammond commented Dec 7, 2016

cibernox commented Dec 7, 2016

stefanpenner left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cibernox commented Dec 12, 2016

Turbo87 commented Dec 12, 2016

stefanpenner commented Dec 12, 2016 • edited Loading

hzoo commented Dec 12, 2016

cibernox commented Dec 12, 2016

hzoo commented Dec 12, 2016 • edited Loading

cibernox commented Dec 12, 2016

cibernox commented Mar 13, 2017

rwjblue commented Mar 13, 2017

rwjblue commented Apr 19, 2017

cibernox commented Apr 19, 2017 • edited Loading

cibernox commented Apr 21, 2017

stefanpenner Dec 12, 2016 •

edited

Loading

stefanpenner left a comment •

edited

Loading

stefanpenner commented Dec 12, 2016 •

edited

Loading

hzoo commented Dec 12, 2016 •

edited

Loading

cibernox commented Apr 19, 2017 •

edited

Loading