fix(gatsby): correctly pick up browserslist overrides #9669
Conversation
| @@ -25,16 +25,14 @@ function preset(context, options = {}) { | |||
| node: `current`, | |||
| } | |||
| } else { | |||
| targets = { | |||
| browsers: pluginBabelConfig.browserslist, | |||
There was a problem hiding this comment.
Note: this will be removed in later version in favor of just setting "targets" to a query directly.
|
|
||
| let siteInfo = { directory, browserslist: DEFAULT_BROWSERS } | ||
| const useYarn = existsSync(path.join(directory, `yarn.lock`)) | ||
| if (isLocalSite) { | ||
| const json = require(path.join(directory, `package.json`)) | ||
| siteInfo.sitePackageJson = json | ||
| siteInfo.browserslist = json.browserslist || siteInfo.browserslist | ||
| siteInfo.browserslist = getBrowsersList(directory, DEFAULT_BROWSERS) |
There was a problem hiding this comment.
This seems kind of weird to have the set up working like this, i.e. gatsby-cli sets up some program stuff, and then other plugins use it. Might try to improve this.
There was a problem hiding this comment.
Yeah, can't remember why gatsby-cli does this
DSchau
changed the title
DSchau
changed the title
|
Alright, so this PR is ready, although I plan to make the improvements to not requiring browserslist in gatsby-cli--which seems weird. After testing, this change does work like we'd expect, but it appears there's at least one weird thing. We use Currently the only check we were making was with the users package.json However - the gotcha is that we don't parse node_modules with babel. This means that useBuiltIns can't know what polyfills we could need to include for e.g. some third party dep. It may be worth doing that in the future, but it doesn't seem like other projects are doing that quite yet. Might be nice to make this opt-in with a plugin or something in the future. |
|
Just need to fix the tests 😅 Doing that now. |
|
CRA v2 now compiles node_modules facebook/create-react-app#1125 |
|
@KyleAMathews I knew I saw something about that! That's probably slightly out of scope for this PR, but that could be worth following a similar approach to how they've done it! |
|
Sounds good! |
|
Also @souporserious this may fix the issue you were running into in development mode! It's kinda hard to validate Windows stuff 😅 Any interest in checking it out? |
|
|
* fix: start ironing out browserslist issues * chore: make linter happy * chore: change to expected shape * refactor: move browserslist functionality into gatsby core
Just to get some 👀 on this early, this is somewhat working. It's fairly challenging to test this, so I've been using
gatsby-plugin-webpack-bundle-analyzerand inspecting the size of the bundled assets. Currently, can't seem to get it to change with the browserslist query, but it is picked up (correctly) and added to.cache/babelState.json.More to do here 🙃
Begins to fix #7003, in that enabling browserslist functionality should clean up IE specific stuff.