Skip to content
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

Respect "browserslist" config #125

Merged
merged 7 commits into from
Jun 23, 2017
Merged

Respect "browserslist" config #125

merged 7 commits into from
Jun 23, 2017

Conversation

lukeed
Copy link
Member

@lukeed lukeed commented Jun 20, 2017

I never saw a way to write to env.browsers (within babel config) aside from a CLI string, which is very error prone as it passes thru the split().

This will share a browsers config across Babel and Autoprefixer. I don't believe there are any other configs...?

Lastly, "upgrades" the default Autoprefixer from 'last 2 versions' to what Babel was specifying.

Closes #124

@lukeed lukeed requested a review from rkostrzewski June 20, 2017 18:43
@@ -4,12 +4,8 @@ export default (env, options={}) => ({
[require.resolve('babel-preset-env'), {
loose: true,
modules: options.modules || false,
browsers: options.browsers,
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️ options.browsers - babel-config.js will be still usable as a babel preset!

However it should be: target: { browsers: [...] } like here (I've wanted to check what this does - I still don't know, but at least I've noticed it is passed differently 😛 )

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah, nice catch! Looks like that should have always been target.browsers 👍

Sorry, I'm confused by your first sentence. Not sure what you mean.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just that I really dig that you pass browsers via options 😛 . That means I'll still be able to us babel-config.js as a babel preset in #56 and use it like so: presets: [path.resolve(..., 'babel-config.js', { browsers: ... } ]

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, haha, great!

I'm thinking now, to answer my own question (below), that I need to respect any "babel" config inside the package.json.

With #56, how will users customize their babel options? I should account for that too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Via .babelrc or package.json (or even by changing webpack babel-loader query). I don't think you have to worry about that at all because babel is so smart that it extends over this configuration, i.e. a .babelrcthat looks like this { plugins: ["my-awesome-plugin"] } will get everything applied from this file + 'my-awesome-plugin'.

In case of collisions stuff from .babelrc takes precedence over our internal settings (don't remember if options were merged tough, e.g. when env preset was declared stuff like loose: true was preserved or in this case browsers would be preserved)

@rkostrzewski
Copy link
Collaborator

I think this should be described it in a readme or some other form of docs.

@lukeed
Copy link
Member Author

lukeed commented Jun 20, 2017

I'll add a README note as part of this PR.

Question: How do we want to allow devs to specify Babel targets? See full usage.

Follow up questions here: #124 (comment)

rkostrzewski
rkostrzewski previously approved these changes Jun 20, 2017
README.md Outdated

#### Browserlist

You may customize your list of supported browser versions by declaring a [`"browerslist"`](https://github.com/ai/browserslist) key within your `package.json`. Changing these values will modify your JavaScript (via [`babel-preset-env`](https://github.com/babel/babel-preset-env#targetsbrowsers)) and your CSS (via [`autoprefixer`]()) output.
Copy link
Collaborator

Choose a reason for hiding this comment

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

browerslist -> browserslist

Copy link
Member Author

Choose a reason for hiding this comment

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

//facepalm 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're on a roll! broswerslist 🤣

rkostrzewski
rkostrzewski previously approved these changes Jun 20, 2017
@lukeed
Copy link
Member Author

lukeed commented Jun 20, 2017

Confession: I hate typing the word "browser" -- even though I know how to spell it, it rarely comes out right & typos blend in. Good thing I'm a front-end dev, eh?

README.md Outdated

#### Browserslist

You may customize your list of supported browser versions by declaring a [`"browserslist"`](https://github.com/ai/browserslist) key within your `package.json`. Changing these values will modify your JavaScript (via [`babel-preset-env`](https://github.com/babel/babel-preset-env#targetsbrowsers)) and your CSS (via [`autoprefixer`]()) output.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: missing link to autoprefixer 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm tired.

@lukeed
Copy link
Member Author

lukeed commented Jun 20, 2017

Thanks @rkostrzewski 😆

Pinging @developit for quick sign-off on README changes

Copy link
Member

@developit developit left a comment

Choose a reason for hiding this comment

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

Awesome.

@@ -7,7 +7,7 @@ charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[{*.json,.*rc,*.yml}]
[{*.json,.*rc,*.yml,*.md}]
Copy link
Member

Choose a reason for hiding this comment

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

weird this wasn't already in here

@developit developit added this to the 1.2.0 milestone Jun 21, 2017
@lukeed
Copy link
Member Author

lukeed commented Jun 23, 2017

Is this ready to make its way in?

@developit
Copy link
Member

IMO yes. and @rkostrzewski reviewed previously but our settings killed it. merging.

@developit developit merged commit 8db82b6 into master Jun 23, 2017
@lukeed lukeed deleted the browserlist branch June 23, 2017 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Honor browserslist in package.json for Babel, Autoprefixer, etc.
3 participants