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

Allow defaults query in preset-env #8897

Merged
merged 1 commit into from Jun 30, 2019
Merged

Allow defaults query in preset-env #8897

merged 1 commit into from Jun 30, 2019

Conversation

existentialism
Copy link
Member

Q                       A
Fixed Issues? Fixes #7405
Patch: Bug Fix? Y
Major: Breaking Change? N
Minor: New Feature? N
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

@existentialism existentialism added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: preset-env labels Oct 18, 2018
@babel-bot
Copy link
Collaborator

babel-bot commented Oct 18, 2018

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9288/

@nicolo-ribaudo
Copy link
Member

Is there a feature which shouldn't be transformed when using Browserlist's default target? If so, we could add it to the tests, to test the difference between "default targets" and "no targets"

@existentialism
Copy link
Member Author

@nicolo-ribaudo part of the issue is, while we can handle usage of "defaults" when the query is passed directly as an option to the preset, if a user relies on browserslist to resolve the config (like via .browserslistrc or package.json) then our behavior of "no targets means the same as preset-latest" is difficult to do since we wont' be able to know whether the user didn't have targets or wants defaults.

For now, seems like just documenting this difference seems okay?

@danez
Copy link
Member

danez commented May 8, 2019

I guess the best solution would be to check ourself if there is a browserlist config in the fs if there are not targets specified. We could do this with browserlist.findConfig(from). If we find one we do not change defaults. But that would also be a breaking change so I guess we would need to wait for v8. https://github.com/browserslist/browserslist/blob/master/node.js#L238

@nicolo-ribaudo nicolo-ribaudo added this to the v7.5.0 milestone May 8, 2019
@nicolo-ribaudo
Copy link
Member

I agree, we should do that in the next major release

@nicolo-ribaudo nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label May 8, 2019
@nicolo-ribaudo nicolo-ribaudo merged commit 77fd7cd into master Jun 30, 2019
@existentialism existentialism deleted the issue7405 branch July 2, 2019 15:02
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env PR: Bug Fix 🐛 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

defaults browserslist query breaks preset-env
5 participants