fix(gatsby): use separate eslint-loader for rules that are always required #29317
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gatsbot
bot
added
the
status: triage needed
vladar
removed
the
status: triage needed
vladar
commented
Feb 3, 2021
Comment on lines
+350
to
+355
| if ( | ||
| isCustomEslint && | ||
| process.env.GATSBY_HOT_LOADER === `fast-refresh` | ||
| ) { | ||
| configRules = configRules.concat([rules.eslintRequired()]) | ||
| } |
There was a problem hiding this comment.
Basically the main piece. We add another minimal loader only when there is user-defined eslint config. When there is no custom config - we still use a single default Gatsby loader (with fast-refresh rules added to it)
pieh
approved these changes
Feb 3, 2021
There was a problem hiding this comment.
I tried testing as much combinations from reports as possible - with gatsby-plugin-eslint - both v2 (that use eslint-loader) and v3 (that use eslint-webpack-plugin) and without
using optional chaining (for old parser), using eslint-disable with "not existing rules" and from what I can tell I had expected results, so let's merge this - at very least this will fix known issues.
Huge thanks!
|
Published in |
This was referenced Feb 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fast-refresh as a hot-reloading mechanism has several constraints. And we need to enforce those constraints using eslint and custom eslint rules. Originally we've implemented this in #28911. And the idea was to merge those rules into a custom user eslint config.
This has caused several issues. This PR suggests a different approach by introducing a separate
eslint-loaderfor fast-refresh rules that is completely isolated from the user custom eslint setup.This loader is only added when there is a custom .eslintrc`. For default Gatsby eslint config we still have a single loader (and just add fast-refresh rules to it).
Documentation
https://www.gatsbyjs.com/docs/how-to/custom-configuration/eslint/
(this PR warrants updating those docs a bit)
Related Issues
An alternative approach to #28911
Fixes #29105
Fixes #29133