Gatsby: [v2] Eslint rules can produce false positives

[jquense]

I was jsut upgrading the react-bootstrap gatsby version and suddenly got a lot of webpack eslint-loader complaints about files where I've specified globals as existing, (e.g. React and the bootstrap components) but the eslint-loader ignores my local .eslintrc file and complains that react is not in scope and that the components are undefined.

Not sure the best way to handle this but it's pretty annoying :P

[jquense]

I think there probably should be an easy way to disable the built in eslint checking...I find it very frustrating that things fail the build that may or may not be errors, especially when i'm already using eslint myself, i don't need gatsby to be doing it on top of my own checks.

Maybe Gatsby should default disable its eslint checking if it detects a local config?

[pieh]

Would changing builtin eslint checking to use local .eslintrc (if it exists) fix this and be the way to go?

/cc @kkemple

[jquense]

yes and no, you still don't have control over how Gatsby responds to lint errors, and a local eslintrc usually contains a lot more style/arbitrary rules. I tried out this change locally but it was frustrating because I had to fix all eslint errors, including ones that aren't bugs (like unused variables or something) before Gatsby would even build and let me look at the site. This really frustrating when you are developing and aren't focused on making sure every change matches the project's styleguide (something i usually do at the end when i'm sure the code is not gonna change much more)

[KyleAMathews]

I think in general following CRA's approach to lint aggressively against things that cause errors is a good idea. Even if it is pretty annoying in the short-term. When running this for gatsbyjs.org — there was dozens of errors 😅

Being able to ignore global errors does seem like something that we should support.

[jquense]

the issue tho is they aren't errors. In the one case eslint is wrong (the default gatsby setup), but failing to let me work. in the case where we use a local eslintrc, its nothing like CRA because it'm controling the rules.

Overall CRA's linting is tolerable because they completely control the environment so they know they won't produce false positives. Gatsby is in a very different place, as a user i can control the webpack configuration, but the eslint config assumes I can't, and assumes it knows everything about my environment, which is not correct. Case in point trying to upgrade a site to the version that included linting, left me in a place where i could not get the site to build because eslint insisted something was broken when it wasn't.

[jquense]

not to mention tho that the CRA eslint config _does_ contain opinions, like it requires I preface globals with window. which I agree with as a good practice and a possible point of error, but I don't think Gatsby as a tool should be deciding that style rule for me.

these are the sort of things that should exist higher up the stack, like in a theme for getting started quickly

[KyleAMathews]

hmmm... yeah, great points. Any suggestions about what we should do? Perhaps add a config option to switch to using a local .eslintrc? Though I hate adding config options :-(

@kkemple any thoughts?

[kkemple]

as @jquense mentioned even switching to local is still going to make webpack stop compiling until everything in local is resolved

I think the option would be to disable linting at the webpack level altogether and leave it in the hands of the app developer.

Which is another reason to make our eslint config sharable (which will be hard with schema support but can be figured out) so then in userland anyone can still opt in to the basic rules by extending the shared gatsby config

another possible option is a plugin that overrides gatsby eslint config (schema could still be a problem here but if opting out, then losing schema linting probably isn't a concern)

[KyleAMathews]

I think 90%+ of Gatsby users barely know what Eslint is so shipping a default config that prevents errors is a good thing.

But since Gatsby is intended to be overridable and used by advanced developers — making it easy to opt out is also very good.

So yeah, a plugin sounds like a pretty reasonable thing — it could include a basic .eslintrc config to replicate what's in core. @kkemple wasn't the plan to start writing out the schema so a local graphql eslint plugin would still work?

[kkemple]

yeah, I think there may be a few unknowns left there (at least for me) but it's definitely step 1 in getting schema linting everywhere

[jquense]

My initial thought was to twofold:

• detect a local eslint config (either in the root, or package.json), if one exists don't do any linting, (except maybe gatsby specfic things like valudate the graphql stuff, or catch gatsby specfic errors). The odds are that if someone has an eslint setup, they don't need help catching errors, their config will almost definately cover the basic JS gotchas

• Allow opting out of any linting easily, for advanced cases and as an escape hatch.

For 1, we'd definitely benefit from extracting the Gatsby stuff so folks and easily add it to their own eslint configs 👍

[KyleAMathews]

I like that — just disable our eslint-loader altogether if someone has something setup locally.

[kkemple]

Yeah, that makes a ton of sense. And once we export our config properly it can be easily re-created as well.

[brad-jones]

It would still be nice to be able to completely disable the linting. Linting and building are 2 separate things. Just because something doesn't lint doesn't mean it shouldn't build. As has been explained above.

We don't have an eslint config, we mostly use typescript but our gatsyby sites are in plain js (for now) which consume many other typescript projects and we get many linting errors on the transpiled typescript code.

For now I have added a blank .eslintrc file which I guess effectively disables the linting, it would just be cleaner if there was a simple config option to opt out.

I haven't tried this yet, what will happen when we do start using gatsby-plugin-typescript will it still want to run eslint? If anything it should probably run tslint... but again I would rather have gatsby just do the gatsby things and then as a more advanced developer configure my CI/CD pipeline to run any linting as required.