Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

chore(babel): added babel to support es6 #10517

Closed
wants to merge 1 commit into from
Closed

chore(babel): added babel to support es6 #10517

wants to merge 1 commit into from

Conversation

EladBezalel
Copy link
Member

No description provided.

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Mar 20, 2017
devversion
devversion approved these changes Mar 20, 2017
View reviewed changes
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES2015 🎉

@Frank3K
Copy link
Contributor

Frank3K commented Mar 22, 2017

What about babel-preset-env? It seems like that is the recommendation now.

@EladBezalel
Copy link
Member Author

EladBezalel commented Mar 22, 2017 •
edited
Loading

@Frank3K I certainly thought of it but i realized it shouldn't be env based..
But now i'm thinking of it again and maybe we should do

"env", { "targets": { "browsers": ["last 2 versions"] } }

@Frank3K
Copy link
Contributor

Frank3K commented Mar 23, 2017 •
edited
Loading

I think you should use the list that is used by autoprefixer (in scripts/gulp-utils.js).

Please note that there are issues with the current version of UglifyJS, as is documented on babel-preset-env.

@EladBezalel
Copy link
Member Author

@devversion what do you think?

@devversion
Copy link
Member

@EladBezalel I think that the babel-preset-env would work. Also like the idea of sharing the browsers array with the one from the autoprefixer.

@Frank3K
Copy link
Contributor

Frank3K commented Mar 26, 2017

Note that you can add the browserlist to package.json, or set it in a separate config file as described here. Autoprefixer and babel-env-preset will automatically pick it up.

@EladBezalel
Copy link
Member Author

@Frank3K @devversion please review

Frank3K
Frank3K reviewed Mar 27, 2017
View reviewed changes
gulp/util.js
@@ -48,6 +49,9 @@ function buildJs () {

var jsBuildStream = gulp.src( jsFiles )
.pipe(filterNonCodeFiles())
.pipe(babel({
presets: [["env", {targets: {browsers: config.browsers}}]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think uglify: true should be added here. Readme and PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds like useBuiltIns would work better

Copy link
Contributor

@Frank3K Frank3K Mar 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand it correctly useBuiltIns alone won't solve the minification issues. I'm not sure what uglify + useBuiltIns does.

The option useBuiltInsseems to require a module loader (see explanation).

@ThomasBurleson
Copy link
Contributor

This is look good. Let's finish with final changes and one last review.

@ThomasBurleson ThomasBurleson added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Mar 27, 2017
@ThomasBurleson
Copy link
Contributor

Closing as not valid for stabilization.

@EladBezalel EladBezalel deleted the chore/es6 branch June 13, 2017 14:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Frank3K Frank3K Frank3K left review comments

@devversion devversion devversion approved these changes

@ThomasBurleson ThomasBurleson Awaiting requested review from ThomasBurleson

Assignees
No one assigned
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@EladBezalel @Frank3K @devversion @ThomasBurleson @googlebot