Skip to content

Conversation

@jonathantneal
Copy link
Contributor

What kind of change does this PR introduce?

  1. This removes manual browserslist configuration detection, allowing browserslist to manage its own configurations.
  2. This change also adds support for all browserslist configuration files. See https://github.com/browserslist/browserslist#config-file
  3. This change also adds support for environment configurations, which would have been previously breaking to Preact CLI, even when done within package.json. See https://github.com/browserslist/browserslist#configuring-for-different-environments

Did you add tests for your changes?

No, this does not add tests, and there were no browserslist tests previously. Also, adding breaking browserslist configurations for the current release would only be useful if this project continues to manually check for configurations, which this change removes. Also, testing browserslist configurations could be problematic, as browserslist-enabled tooling within Babel and PostCSS would frequently change output as polyfills are updated and global usage stats change.

Summary

I looked into this codebase to see why our .browserslistrc file was not being respected.
I saw the excellent work in #125 and decided to bring it that effort further into alignment with browserslist with this PR.

Does this PR introduce a breaking change?

No, this does not introduce a breaking change.

Other information

This adds a browserslist dependency, but it should de-dupe along with the other nested browserslist dependencies. See these results for nom ls browserslist:

[email protected] /preactjs/preact-cli/packages/cli
├─┬ @babel/[email protected]
│ ├── [email protected]  deduped
│ └─┬ [email protected]
│   └── [email protected]  deduped
├─┬ [email protected]
│ └── [email protected]  deduped
├── [email protected] 
└─┬ [email protected]
  └─┬ [email protected]
    └─┬ [email protected]
      ├─┬ [email protected]
      │ └── [email protected]  deduped
      ├─┬ [email protected]
      │ └─┬ [email protected]
      │   └── [email protected]  deduped
      ├─┬ [email protected]
      │ ├── [email protected]  deduped
      │ └─┬ [email protected]
      │   └── [email protected]  deduped
      ├─┬ [email protected]
      │ └── [email protected]  deduped
      ├─┬ [email protected]
      │ └── [email protected]  deduped
      └─┬ [email protected]
        └── [email protected]  deduped

Copy link
Member

@developit developit left a comment

Choose a reason for hiding this comment

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

LGTM!

@developit developit requested a review from ForsakenHarmony July 9, 2019 14:39
@prateekbh prateekbh merged commit 7df92f9 into preactjs:master Jul 10, 2019
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.

4 participants