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

[WIP] Add ecmascript target to babel-preset-env #6731

Open
wants to merge 13 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@rtsao
Contributor

rtsao commented Nov 2, 2017

Q                       A
Fixed Issues? Fixes #6613
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR
Any Dependency Changes? No
License MIT
@babel-bot

This comment has been minimized.

Collaborator

babel-bot commented Nov 2, 2017

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8587/

@rtsao rtsao changed the title from WIP: Add ecmascript target to babel-preset-env to Add ecmascript target to babel-preset-env Nov 2, 2017

@xtuc

Thanks for your PR 👍

@@ -57,7 +57,7 @@
"@babel/core": "7.0.0-beta.31",
"@babel/helper-fixtures": "7.0.0-beta.31",
"@babel/helper-plugin-test-runner": "7.0.0-beta.31",
"compat-table": "kangax/compat-table#957f1ff15972e8fb2892a172f985e9af27bf1c75",
"compat-table": "https://api.github.com/repos/kangax/compat-table/tarball/957f1ff15972e8fb2892a172f985e9af27bf1c75",

This comment has been minimized.

@xtuc

xtuc Nov 7, 2017

Member

I'm wondering why you changed this. Did you encounter an issue? I don't think we should use the API directly since we will likely be rate limited.

This comment has been minimized.

@rtsao

rtsao Nov 7, 2017

Contributor

This was due to a yarn-related bug, which I think may actually be fixed in the latest version. Let me try again

This comment has been minimized.

@rtsao

rtsao Nov 7, 2017

Contributor

Alright, I've reverted this change and rebased to latest master

This comment has been minimized.

@xtuc

xtuc Nov 7, 2017

Member

Thanks.

@@ -187,6 +187,12 @@ Example environments: `chrome`, `opera`, `edge`, `firefox`, `safari`, `ie`, `ios
The [data](https://github.com/babel/babel-preset-env/blob/master/data/plugins.json) for this is generated by running the [build-data script](https://github.com/babel/babel-preset-env/blob/master/scripts/build-data.js) which pulls in data from [compat-table](https://kangax.github.io/compat-table).
### `targets.ecmascript`
` "5" | "2015" | "2016" | "2017" | "2018"`

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Nov 13, 2017

Member

Also 3? Nevermind, it is not supported by preset-env

@rtsao

This comment has been minimized.

Contributor

rtsao commented Dec 13, 2017

This PR makes it much easier for library authors to explicitly support a certain minimum ECMAscript version, or even publish multiple versions of their code (targeting different ECMAScript specs). I think this is a nice alternative to requiring consumers to run arbitrary transformations deeply in node_modules.

Any thoughts on this, @hzoo @loganfsmyth @existentialism ?

@@ -187,6 +187,12 @@ Example environments: `chrome`, `opera`, `edge`, `firefox`, `safari`, `ie`, `ios
The [data](https://github.com/babel/packages/babel-preset-env/blob/master/data/plugins.json) for this is generated by running the [build-data script](https://github.com/babel/packages/babel-preset-env/blob/master/scripts/build-data.js) which pulls in data from [compat-table](https://kangax.github.io/compat-table).
### `targets.ecmascript`
` "5" | "2015" | "2016" | "2017" | "2018"`

This comment has been minimized.

@yavorsky

yavorsky Jan 5, 2018

Member

Since we support shippedProposals, should we add a stage3/shippedProposals option?

This comment has been minimized.

@rtsao

rtsao Jan 5, 2018

Contributor

I think the problem of that is it changes over time (especially for the published package use case). When targeting specific browsers or ECMAScript versions, the output would always be the same.

This comment has been minimized.

@yavorsky

yavorsky Jan 6, 2018

Member

@rtsao Makes sense 👍

if (source === "es6") {
ecmascript = 2015;
} else if (source === "es2016plus") {
if (category.match(/2016/)) {

This comment has been minimized.

@yavorsky

yavorsky Jan 5, 2018

Member

Let's prevent match invocation more than once for 2017 and 2018 cases?
For example,

const esVersion = category.match(/201[6-8]/)
if (!esVersion) throw new Error('Could not find ecmascript...')
ecmascript = +esVersion[0];
}
return (
parseInt(lowestImplementedVersion, 10) >
parseInt(lowestTargetedVersion, 10)

This comment has been minimized.

@yavorsky

yavorsky Jan 5, 2018

Member

Hopefully, ECMAScript 2019 won't be named ES X 😄

@@ -49,6 +49,8 @@ const pluginListWithoutProposals = filterStageFromList(
proposalPlugins,
);
const validEcmascriptTargets = new Set(["5", "2015", "2016", "2017", "2018"]);

This comment has been minimized.

@yavorsky

yavorsky Jan 5, 2018

Member

Also, we can have a single source of truth and consider this Set for build-data script. Or vice versa. It could reduce steps needed to add a new version in the future.

This comment has been minimized.

@rtsao

rtsao Jan 5, 2018

Contributor

Good idea

This comment has been minimized.

@rtsao

rtsao Jan 6, 2018

Contributor

After investigating the implementation of this, I worry about this being overly abstracted, especially since the data source isn't that consistent and heuristics can be a bit sub-optimal. If the format of the compat-tables change, you might be left with a bad abstraction, so I'm not sure sacrificing clarity/simplicity for DRY would be the right call.

@yavorsky

This comment has been minimized.

Member

yavorsky commented Jan 5, 2018

I haven't strong confidence about that and there will be not much use cases. It's better to know needed targets instead of just es version, which not optimized for common cases.

For example, we have all modern browsers (and stable node) which support es2015-es2017. And ie.. which doesn't.

By using specific targets we could more accurately specify what plugins to avoid. Even ie will ignore some of es2015 transformations and safari TP - some of stage-3. And having es2015 or es2017 will be the not optimized solution neither for ie nor for modern browsers (chrome, ff, safari), which are evergreen by default.

But on the other hand, the idea to publish multiple bundles for library authors is cool!
Get untransformed react with import 'react/es2017' could avoid some transformation bugs and reduce total bundle size! Maybe someday we'll have a tool that resolves this by watching your current preset-env targets and get an optimized package version from node_modules.

@rtsao rtsao referenced this pull request Jun 18, 2018

Open

RFC: Better babel-preset-env API #8195

2 of 5 tasks complete

@rtsao rtsao changed the title from Add ecmascript target to babel-preset-env to [WIP] Add ecmascript target to babel-preset-env Jul 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment