ember-cli / rfcs

Archive of RFCs for changes to ember-cli (for current RFC repo see https://github.com/emberjs/rfcs)
45 stars 54 forks source link

Make addons honour app's transpilation by default #89

Closed cibernox closed 7 years ago

cibernox commented 7 years ago

rendered

Turbo87 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

cibernox commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

rwjblue commented 7 years ago

yes, but not the specific API's suggested IIRC

rwjblue commented 7 years ago

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

cibernox commented 7 years ago

@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 commented 7 years ago

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