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

Move browsers to cross-tool config #3644

Merged
merged 1 commit into from
Jan 9, 2018
Merged

Move browsers to cross-tool config #3644

merged 1 commit into from
Jan 9, 2018

Conversation

ai
Copy link
Contributor

@ai ai commented Dec 26, 2017

Hi. I moved browsers from Autoprefixer’s inner option to browserslist in package.json.

Reason

  • Autoprefixer team recommends only package.browserslist as a way to set target browsers.
  • package.browserslist is used in many different tools. It is used by autoprefixer, stylelint-no-unsupported-browser-features, eslint-plugin-compat, postcss-cssnext, postcss-preset-env, postcss-normalize. The most important, this config will be used in next babel-preset-env 7.0.
  • It is used by autoprefixer-info CLI tool. This CLI tool is part of autoprefixer package, but didn’t work correctly without Browserslist config.
  • This config is useful not only for tools. The new developer will have the single place to check target browsers in a new project.
  • CSS-Tricks agree that Browserslist config is really good idea.

Test

  1. Create a new project.
  2. Check project’s package.json for browserslist key.

@Timer Timer added this to the 2.0.0 milestone Dec 27, 2017
@Timer
Copy link
Contributor

Timer commented Jan 2, 2018

We need to confirm the effect this has on babel-preset-env and we'd still like a way to limit the directory-up search to stop at the project boundary (we don't want a config file in the user's home directory to be picked up).

@ai
Copy link
Contributor Author

ai commented Jan 2, 2018

Only babel-preset-env 7.0 will support it. But we have several tools with Browserslist already. Should we enable it right now and on Babel 7 release it will be used for babel-preset-env,?

@ai
Copy link
Contributor Author

ai commented Jan 3, 2018

@Timer how do you suggest to limit directory-up search?

We need a really safe way since Browserslist is used in Autoprefixer (and it is the only recommended way to set browsers) with 10 M users :(.

We can limit it with first directory with .git/ inside. Does it safe? Does it used in other projects with directory-up search?

What limit is used in .babelrc and .eslintrc search?

@Timer
Copy link
Contributor

Timer commented Jan 3, 2018

I'm not sure of the search depth, but we disable .babelrc and .eslintrc usage.
Do you think we could disable browserslist search and provide it explicitly instead (by reading it ourselves)?
I believe this is the better option. This way we could inject a good default value as well.


If directory up search limit would be an option, I'd imagine you'd provide a path to stop at as a configuration variable.

@ai
Copy link
Contributor Author

ai commented Jan 3, 2018

@Timer yeap, we can read browserslist queries from some specific place. I can change PR for this behavior. What place will the best? browserslist key in ./package.json or .browserslistrc config?

@Timer
Copy link
Contributor

Timer commented Jan 3, 2018

Hmm, probably strictly browserslist in package.json while not-ejected, once ejected it can revert to default behavior and read from .browserslistrc and via directory-up method.

@gaearon gaearon modified the milestones: 2.0.0, 1.0.18 Jan 9, 2018
@gaearon
Copy link
Contributor

gaearon commented Jan 9, 2018

Meh. I don't feel too strongly about upwards search here—doesn't seem that it will be as common a problem as Babel configs.

@gaearon
Copy link
Contributor

gaearon commented Jan 9, 2018

Is this change backwards-compatible? What happens for existing users who don't have anything in package.json? Ideally I'd like it to use our current list in this case.

@gaearon gaearon changed the base branch from master to next January 9, 2018 16:38
@gaearon gaearon modified the milestones: 1.0.18, 2.0.0 Jan 9, 2018
@gaearon gaearon merged commit cbf8d79 into facebook:next Jan 9, 2018
@gaearon
Copy link
Contributor

gaearon commented Jan 9, 2018

I started a branch for 2.0, getting it in. This is a breaking change because existing projects will need to add browserslist to avoid the new defaults (which now match BL's own defaults).

gaearon pushed a commit that referenced this pull request Jan 9, 2018
@Timer Timer added this to Ready in 2.0 Jan 10, 2018
gaearon pushed a commit that referenced this pull request Jan 10, 2018
Timer pushed a commit to Timer/create-react-app that referenced this pull request Jan 11, 2018
Timer pushed a commit that referenced this pull request Jan 11, 2018
Timer pushed a commit to Timer/create-react-app that referenced this pull request Jan 11, 2018
Timer pushed a commit to Timer/create-react-app that referenced this pull request Jan 13, 2018
gaearon pushed a commit that referenced this pull request Jan 13, 2018
Timer pushed a commit that referenced this pull request Jan 14, 2018
gaearon pushed a commit that referenced this pull request Jan 14, 2018
gaearon pushed a commit that referenced this pull request Jan 14, 2018
gaearon pushed a commit that referenced this pull request Jan 14, 2018
gaearon pushed a commit that referenced this pull request Jan 14, 2018
Timer pushed a commit to Timer/create-react-app that referenced this pull request Jan 15, 2018
akstuhl pushed a commit to akstuhl/create-react-app that referenced this pull request Mar 15, 2018
@frederikhors
Copy link

@gaearon @ai is this still valid for 2?

@ai
Copy link
Contributor Author

ai commented Sep 27, 2018

Yeap, but not for Babel.

@gaearon maybe we should explain it more clear in changelog? With a link to Browserslist docs. Not a first-time people asking.

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

I'll add it to the final release documentation checklist.

zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
2.0
  
Ready
Development

Successfully merging this pull request may close these issues.

None yet

5 participants