Skip to content

Make browsers list configurable - #2358

Closed
cr101 wants to merge 25 commits into
react:masterfrom
cr101:master
Closed

Make browsers list configurable#2358
cr101 wants to merge 25 commits into
react:masterfrom
cr101:master

Conversation

@cr101

Copy link
Copy Markdown
Contributor

Autoprefixer's browsers option is currently hardcoded. This PR adds the browserslist key in package.json which allows developers to set different browsers for both production and development.
Autoprefixer which relies on Browserslist will find its config automatically.
This PR fixes#2248 and #892

cr101 added 2 commits May 25, 2017 15:58
…he browserslist key in package.json which allows developers to set browsers list in queries like "Chrome >= 35", "Firefox >= 38" for both production and development. Autoprefixer which relies on Browserslist will find its config automatically. This PR fixesreact#2248 and react#892
@gaearon

Copy link
Copy Markdown
Contributor

Does this let me specify browserslist in package.json of unejected apps? We should support that (with our hidden default if it’s unspecified).

@gaearongaearon added this to the 2.0.0 milestone May 25, 2017
@cr101

Copy link
Copy Markdown
ContributorAuthor

@gaearon

Does this let me specify browserslist in package.json of unejected apps?

Yes. Running create-react-app my-app will add the default browserslist in package.json as below:

screenshot_6

@gaearon

Copy link
Copy Markdown
Contributor

Do you think it would make sense to use a single list by default, and document splitting dev/prod in user guide? Not sure if it would be confusing to most people or not.

I also think we should still provide a default for people who don't have a browserslist in their package.json. Since they probably will forget to add it upgrading. We should either force it to be there, or provide a good default.

@cr101

Copy link
Copy Markdown
ContributorAuthor

@gaearon I agree that if people forget to add browserslist to their package.json when they upgrade then we should force it to be there. If you agree with the approach then I will do the user documentation next.

Do you think it would make sense to use a single list by default, and document splitting dev/prod in user guide? Not sure if it would be confusing to most people or not.

A number of people have asked to split dev/prod

@gaearon

gaearon commented May 30, 2017

Copy link
Copy Markdown
Contributor

I'd like to split these changes if you don't mind.

Let's start with using default values when browserslist is not specified. We can force it to be specified in a later release, but I'd rather release this as a non-breaking change for now.

I would also like to support both single array and an object with development and production configurations.

let supportedBrowsers = require(paths.appPackageJson).browserslist;
if (!supportedBrowsers || Object.keys(supportedBrowsers).length === 0) {
console.log(
chalk.yellow(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to not introduce warnings for this.
It will be a straightforward step in migration guide, but for now I don't want people to get annoyed.

@cr101

cr101 commented Jun 29, 2017

Copy link
Copy Markdown
ContributorAuthor

Sorry, I accidentally messed up committing my changes while merging changes from upstream.

@gaearon

Copy link
Copy Markdown
Contributor

Hmm. I wonder if we should count this as a breaking change or not. I guess not.

@gaearongaearon modified the milestones: 1.x, 2.0.0Jun 29, 2017
@Timer

Timer commented Jun 29, 2017

Copy link
Copy Markdown
Contributor

IIRC, browserlist searches all the way up the folder tree. This might cause confusing situations if users have dangling package.jsons (something we've seen before) or malformed ones (it complains loudly when malformed).
Can we limit its search scope?

@cr101

cr101 commented Jul 2, 2017

Copy link
Copy Markdown
ContributorAuthor

@Timer Any idea how to limit the browserlist search scope?

const paths = require('./paths');
const getClientEnvironment = require('./env');

// Get supported browsers list

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this can be moved to a separate module to avoid DRY, maybe config/getSupportedBrowsers?

@Timer

Timer commented Jul 2, 2017

Copy link
Copy Markdown
Contributor

@cr101https://github.com/ai/browserslist/blob/master/index.js#L361 looks like it can't be configured; open an issue and PR maybe?

@cr101

cr101 commented Jul 2, 2017

Copy link
Copy Markdown
ContributorAuthor

@Timerbrowserslist just does a JSON.parse of package.json to find the list of browsers section.

@Timer

Timer commented Aug 3, 2017

Copy link
Copy Markdown
Contributor

What needs done here?
Have we made that pull request to limit the scope searching?

@cr101

cr101 commented Aug 3, 2017

Copy link
Copy Markdown
ContributorAuthor

Have we made that pull request to limit the scope searching?

I don't believe so. I'm now on Windows which makes it tricky to test the changes.
@Timer I'd appreciate your help with limiting the search scope.

@gaearon

Copy link
Copy Markdown
Contributor

Really sorry about this—we were very busy with React 16 rollout and by the time we got around to reviewing #3644 was more fresh so we took it instead. Thanks for prototyping this early on!

@gaearongaearon closed this Jan 14, 2018
@locklockBot locked and limited conversation to collaborators Jan 20, 2019
Sign up for freeto 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.

Option for enabling Flexbox 2009

5 participants

@cr101@gaearon@Timer@viankakrisna@facebook-github-bot