-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Allow for browserslist environment variables #7764
Conversation
It looks like the build is now passing for other MRs, can someone please re-run CI on this MR? |
f6c587e
to
55b7e53
Compare
Never mind, I just pushed an amended commit. |
@GreenGremlin We've had some problems with out CR, sorry that you had to push to get your PR (😉) to pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, can you share the documentation on this? I couldn't find it on their repo.
Unfortunately, just like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look there, thanks! I've tagged this for the next minor.
It's interesting that we're setting this in our e2e tests. I imagine it must not be doing anything? create-react-app/tasks/e2e-kitchensink.sh Line 107 in ac93f31
|
The environment variables work for everything but the check for browserslist config that determines wether we should prompt to add it. Also, if the e2e tests are not run interactively, we may just skip the prompt entirely. |
Thanks @GreenGremlin! |
Browserslist allows for setting browserslist config using environment variables, but CRA still prompts for setting browserslist config when it is already configured for the environment.
For example, running this currently results in CRA improperly detecting that browserslist is not configured, even though it is.
BROWSERSLIST="> 5%" npm run build