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

Throw error on missing environment #560

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sgomes
Copy link

@sgomes sgomes commented Nov 25, 2020

This PR introduces a check for missing environment configuration when explicitly providing an environment name, either by including it in opts or by providing it via the BROWSERSLIST_ENV environment variable.

It now throws an explicit error, whereas before it silently used the config defaults.

As discussed in #559, this is a breaking change.

Fixes #559.

@ai ai added this to the 5.0 milestone Nov 25, 2020
@ai ai changed the base branch from master to main December 1, 2020 04:53
@Semigradsky
Copy link
Contributor

What if add an option about generate or not error in this case?
It can be not breaking change if this option will be disabled by default.
Tools which used browserslist can enable option in theirs major releases but browserslist will remain in 4.x version.

@sgomes
Copy link
Author

sgomes commented Jan 26, 2021

Thanks for taking a look at this PR, @Semigradsky! I'm not sure I follow your suggestion, however. The idea is to only enable this in browserslist 5.x, since it's a breaking change. As such, any tools that use browserslist 4.x will keep the old behaviour, and only tools that get updated to browserslist 5.x will get the new behaviour. This is the correct use of a major version number, in my opinion.

Adding an option seems to me like a bad idea, because it could lead to different browserslist 5.x-enabled tools behaving differently for the same browserslist config and version.

Perhaps I'm misunderstanding your suggestion?

@Semigradsky
Copy link
Contributor

Don't worry, this is just my suggestion on how to release it in the near future)
Releasing a major version of browserslist can be painful.

@ai
Copy link
Member

ai commented Jan 26, 2021

@Semigradsky anyway we need major release for other features

@Muradalsaaideh
Copy link

This PR introduces a check for missing environment configuration when explicitly providing an environment name, either by including it in opts or by providing it via the BROWSERSLIST_ENV environment variable.

It now throws an explicit error, whereas before it silently used the config defaults.

As discussed in #559, this is a breaking change.

Fixes #559.

@Muradalsaaideh
Copy link

This PR introduces a check for missing environment configuration when explicitly providing an environment name, either by including it in opts or by providing it via the BROWSERSLIST_ENV environment variable.

It now throws an explicit error, whereas before it silently used the config defaults.

As discussed in #559, this is a breaking change.

Fixes #559.

111e1f6

@browserslist browserslist deleted a comment Aug 24, 2022
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.

browserslist returns default config for unrecognised environments
5 participants