Skip to content
This repository was archived by the owner on May 11, 2018. It is now read-only.

Add browserslist config/package.json section support. #161

Merged
merged 34 commits into from
Aug 1, 2017

Conversation

yavorsky
Copy link
Member

@yavorsky yavorsky commented Feb 6, 2017

This PR adds support for browserslist field in the package.json and browserslist file (including path option to specify entry point for config file searching).

Starting from v1.5, browserslist supports package.json and browserslist config even earlier.

As @ai mentioned in #108, most of babel-preset-env users also use autoprefixer.
Also, @amilajack created amazing eslint-plugin-compat which detects browsers to support from browserslist query.

It's really easy and painless to use same config value for many things. It allows us simply consider environments, speed up the time to configure a project, unify configs at all.

browserslist doesn't support node.js yet, but it could be added easily in targets. See.

I believe that #114 will come to life and we could use browserslist query for front-end stuff and engines/devEngines for node.js projects.

A few examples..
package.json

"browserslist": "ie 10, > 2%"

or
browserslist

[production]
last 2 version
ie 9

[development]
last 1 version

Note: we had fruitful discsussion with @ai, @existentialism and @amilajack about the default result for empty browserslist query. As I mentioned before, default browsers is a good idea at all, but for now, we must be sure that config-less usage doesn't break anything, so there are no default browsers as well as before, so ignoring browserslist at all won't cause some targets exclusion.

src/index.js Outdated
browserslist.defaults = objectToBrowserslist(targetOps);

const browsersQuery = targetOps.browsers;
const browserslistOpts = {path: opts.path};
Copy link
Member

Choose a reason for hiding this comment

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

Should we rename the browserslist path option on our end to something more descriptive?

Maybe browserslistConfigPath?

Copy link
Member Author

@yavorsky yavorsky Feb 6, 2017

Choose a reason for hiding this comment

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

@existentialism I was thinking about this but decided we could have common path option for all things. For example, while searching package.json for engines in the future. And apparently, it would be the same path for package.json in this case.

I agree that we should pay attention to this, but ideally try not to spread new options, but to unify it and make it works in chime.

If we really could have problems with a common path, browserslistConfigPath is good. Or browserslistPath, because it's also necessary for package.json searching.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, gotcha.

I guess my only real nit with the name "path" is that in a list of options I'd find myself asking "path to what?"

How about configPath? I dunno. Ultimately, it's not a big issue and am fine with it if @hzoo or anyone else doesn't care :)

@darsain
Copy link

darsain commented Feb 6, 2017

If this should eventually accept node versions, shouldn't it be named something environment agnostic, like "envslist"? It will feel weird putting node > 4 into a property named "browserslist".

@yavorsky
Copy link
Member Author

yavorsky commented Feb 6, 2017

@darsain As I mentioned above, browserslist doesn't support node.js currently:
browserslist/browserslist#75

You can specify targets: {node: 7.4} in babel-preset-env's options.

@existentialism
Copy link
Member

@yavorsky woot, babel/babel#4834 merged!

@yavorsky
Copy link
Member Author

@existentialism Wow, thanks!🎉 Going to update path option in 2 hours!

@yavorsky
Copy link
Member Author

@existentialism Pushed but seems like tests will fail for older babel versions which not provide dirname as a 3 argument for presets.

@yavorsky yavorsky requested a review from motiz88 February 23, 2017 11:45
@motiz88
Copy link

motiz88 commented Feb 23, 2017

@yavorsky Needs more tests. I've actually started implementing this before coming across your PR, so should be able to port some of the testing work I've done if you want. It'll take me a couple of days though.

@motiz88
Copy link

motiz88 commented Feb 23, 2017

At any rate, when the new version of babel-core containing babel/babel#4834 gets released, we should depend on it explicitly in CI, and also add a note to the README mentioning this along with the current note on Browserslist.

@yavorsky
Copy link
Member Author

@motiz88 It would be cool! What kind of tests you want to implement. Currently, with fixtures it looks for browserslist config in file's directory and checks whether or not config used like a browserslist query. If config was not found - preset uses targets. And if no targets is specified - works like preset-latest.
Also, I've added unit-test for objectToBrowserslist to parse plain object into string that browserslist understands.

@yavorsky
Copy link
Member Author

@motiz88 Yeah. If we want to cover all things we must be sure that current version of babel-core includes babel/babel#4834.

@existentialism
Copy link
Member

existentialism commented Mar 18, 2017

@yavorsky let's retarget to 2.0, then we can land this w/o worrying about tests failing as dirname would always be present

@yavorsky
Copy link
Member Author

@existentialism It already has 2.0 as a base branch. 🙂 Will resolve conflicts in the morning. Also needs to update readme and discuss the main points before merge (like cases to look up browserslist config, behavior when browsers: [] specified, etc. I will summarize all the things after night (from mobile now 🌙).

@yavorsky yavorsky requested a review from existentialism July 1, 2017 12:40
@existentialism existentialism merged commit 874881d into babel:2.0 Aug 1, 2017
@chicoxyzzy
Copy link
Member

🎉 Can't wait to try new beta with this feature!

@damianobarbati
Copy link

damianobarbati commented Aug 14, 2017

@yavorsky is this merged? Can we finally have the following in package.json and webpack.config.js?

"browserslist": [
    "chrome 58",
    "safari 10"
],
"babel": {
    "presets": [
        [
            "env",
            {
                //no targets because already defined!!!!! <3
                "modules": false,
                "useBuiltIns": true,
                "debug": true
            }
        ],
        "stage-0",
        "react"
    ]
}

And just this in webpack.config.js?

rules: [{
    test: /\.(js|mjs)$/i,
    use: 'babel-loader',
}]

@yavorsky
Copy link
Member Author

@damianobarbati It was merged and you can try it using latest alpha - 2.0.0-alpha.19.

@damianobarbati
Copy link

damianobarbati commented Aug 14, 2017

@yavorsky I just tested this and it works great.

Gotchas for everybody:

  1. some error thrown because of new useBuiltIns usage (https://github.com/babel/babel-preset-env/tree/v2.0.0-alpha.19#usebuiltins-usage).

  2. I still don't get whether is better "usage" or "entry": assuming I'm using webpack, es6 imports and tree shaking which is the best? Is manually importing babel-polyfill still required?

  3. I'm transpiling modules which own a jsnext:main property: problem is this modules are using the old useBuiltIns:true having me to bump versions with fix. Question: shouldn't babel transpile the modules using the "main/parent" babel configurations instead of that one? 🤔

@hzoo
Copy link
Member

hzoo commented Aug 14, 2017

@damianobarbati those questions seem unrelated to this issue/PR and about useBuiltIns, can ask in slack or make a discussion issue about it

@damianobarbati
Copy link

@hzoo correct, moving them where appropriate.
@yavorsky inserted statements like import "babel-polyfill/core-js/modules/es6.map"; are resulting into errors to me: I checked node_modules/babel-polyfill and there's no core-js folder thus the errors.
Weird thing is this happens only with usage and not for entry option.

BTW: is it correct to report this into this commit thread? Or should I open an issue?

@existentialism
Copy link
Member

What version of babel-polyfill are you using?

See:
https://github.com/babel/babel/tree/7.0/packages/babel-polyfill/src/core-js/modules

@damianobarbati
Copy link

solved installing "babel-polyfill": "^7.0.0-alpha.19", thanks

rmacklin added a commit to rmacklin/browserslist that referenced this pull request Sep 24, 2017
Previously this comment read:
> no config support, only tool option

However, this was confusing, because if you follow the link to the
babel-preset-env repository, you will find documentation about support
for external config in package.json or browserlist files:
https://github.com/babel/babel-preset-env/blame/927a3b521907fce260898208a3d30c1694917730/README.md#L108-L149

That's because the master branch of the babel-preset-env repository is
for version 2.0, while the README for the latest 1.x release is only
available on the 1.x branch.

Version 2.0 is currently still in beta, but it includes support for
reading an external config in package.json or browserslist files (via
babel/babel-preset-env#161).

With this in mind, I've updated this comment to:
> external config in package.json or browserslist files supported in v2.0+

in an effort to remove this confusion. Once version 2.0 gets out of
beta, we can probably remove this comment, but until then I think it's
helpful, and lets user's know that if they want external config support
they can try the latest beta release of babel-present-env v2.0.
rmacklin added a commit to rmacklin/browserslist that referenced this pull request Sep 24, 2017
Previously this comment read:
> no config support, only tool option

However, this was confusing, because if you follow the link to the
babel-preset-env repository, you will find documentation about support
for external config in package.json or browserlist files:
https://github.com/babel/babel-preset-env/blame/927a3b521907fce260898208a3d30c1694917730/README.md#L108-L149

That's because the master branch of the babel-preset-env repository is
for version 2.0, while the README for the latest 1.x release is only
available on the 1.x branch.

Version 2.0 is currently still in beta, but it includes support for
reading an external config in package.json or browserslist files (via
babel/babel-preset-env#161).

With this in mind, I've updated this comment to:
> external config in package.json or browserslist files supported in v2.0+

in an effort to remove this confusion. Once version 2.0 gets out of
beta, we can probably remove this comment, but until then I think it's
helpful, and lets users know that if they want external config support
they can try the latest beta release of babel-present-env v2.0.
ai pushed a commit to browserslist/browserslist that referenced this pull request Sep 25, 2017
Previously this comment read:
> no config support, only tool option

However, this was confusing, because if you follow the link to the
babel-preset-env repository, you will find documentation about support
for external config in package.json or browserlist files:
https://github.com/babel/babel-preset-env/blame/927a3b521907fce260898208a3d30c1694917730/README.md#L108-L149

That's because the master branch of the babel-preset-env repository is
for version 2.0, while the README for the latest 1.x release is only
available on the 1.x branch.

Version 2.0 is currently still in beta, but it includes support for
reading an external config in package.json or browserslist files (via
babel/babel-preset-env#161).

With this in mind, I've updated this comment to:
> external config in package.json or browserslist files supported in v2.0+

in an effort to remove this confusion. Once version 2.0 gets out of
beta, we can probably remove this comment, but until then I think it's
helpful, and lets users know that if they want external config support
they can try the latest beta release of babel-present-env v2.0.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.