Should functions contain a .eslintignore file?

[hithomasmorelli]

[REQUIRED] Environment info

firebase-tools: 8.17.0

Platform: macOS Big Sur 11.0.1

[REQUIRED] Test case

See repository at https://github.com/hithomasmorelli/vscode-eslint-parseroptions-project-repro

[REQUIRED] Steps to reproduce

Follow the steps in the README.md file within the repository linked above

[REQUIRED] Expected behavior

As npm run lint is set to only run eslint on "src/**/*", I think it would be expected for (or, at least, I expected) the VS Code ESLint extension to lint those same files.

[REQUIRED] Actual behavior

However, the VS Code ESLint extension seems to work by calling eslint . on the specified working directory. This leads to the following problem to display within VS Code when opening functions/.eslintrc.js (as, if I understand correctly, only the src directory - which does not include .eslintrc.js - is included within tsconfig.json):

I raised issue microsoft/vscode-eslint#1135 in relation to this, but it was closed as the extension working as expected - as the same result was obtained by running eslint . from the terminal.

I then enquired as to whether or not there was a way to tell the VS Code ESLint extension to only match a specified pattern ("src/**/*") within the functions working directory - to which the answer was ".eslintignore".

So, here is the dilemma. Should the default functions setup include an .eslintignore file, in order to prevent VS Code (and any other editors/extensions that may use ESLint in this way) from flagging files that we don't actually intend to be linted? And, if an .eslintignore file is added, and set up so that it excluded everything except for "src/**/*", should the command that is run for npm run lint be changed to just eslint .?

Interested to hear people's thoughts on this.

[samtstern]

@hithomasmorelli thank you for explaining this so clearly!
@bkendall any thoughts about this? You know more about ESLint than most of us

[bkendall]

First: when .eslintignore does not exist, eslint will fall back to using .gitignore. In our templates, all JS files are ignored by default except .eslintrc.js (the condition was added in #2647 - by a familiar author 😉 ). My hunch initially is that is why that file is being checked.

But: I went and tried to figure out why this happens on a new Functions instantiation and not this repository (which follows the same set this pattern). Funny enough, firebase init winds up with eslint@7 installed. If I bring eslint back down to 6, the error you are seeing goes away and I am left with a warning when linting this file specifically

bkend-macbookpro2:functions » ./node_modules/.bin/eslint .eslintrc.js

/private/var/folders/nq/hzrbwr813ndbwnwnd7bbm14400ccww/T/tmp.0pDukbVh/functions/.eslintrc.js
  0:0  error  Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: .eslintrc.js.
The file must be included in at least one of the projects provided

✖ 1 problem (1 error, 0 warnings)

bkend-macbookpro2:functions » ./node_modules/.bin/eslint --version
v7.15.0
bkend-macbookpro2:functions » npm i eslint@6
+ [email protected]

bkend-macbookpro2:functions » ./node_modules/.bin/eslint --version
v6.8.0
bkend-macbookpro2:functions » ./node_modules/.bin/eslint .eslintrc.js

/private/var/folders/nq/hzrbwr813ndbwnwnd7bbm14400ccww/T/tmp.0pDukbVh/functions/.eslintrc.js
  0:0  warning  File ignored by default.  Use a negated ignore pattern (like "--ignore-pattern '!<relative/path/to/filename>'") to override

✖ 1 problem (0 errors, 1 warning)

Sure enough, doing a bunch of digging leads me to this: eslint/eslint#12888. They are now "unignore"-ing .eslintrc.*, which means it's being linted by default with eslint ..

Now onto what to do. I think adding .eslintignore is actually the wrong move here - I would rather have the linter also be validating this file if possible. The thing that we have going for us is that eslintrc.parserOptions.project is a string[] | string, so multiple tsconfigs can be provided that cover all our files. The idea kicking around the top of my head is to add a second tsconfig.dev.json that specifies .eslintrc.js as its only source (with some very, very basic other configuration). That config then can be included into parserOptions.project and eslint will be able to be changed (in package.json to eslint . rather than src/**/* - which in turn will make vscode much happier. There's probably some subtle tinkering that has to happen, but if you start with a clean firebase init folder and then copy the desired end state back into the templates, it should be straight-forward to manage.

(As a side node: the experience between "all-terminal development" and "IDE development" is something that I like keeping the same - all warnings/errors should show up as warnings/errors in both. It's sometimes hard, but totally worth it in the long run.)

Also, this will be something fun for me to check on when I upgrade eslint for firebase-tools, so I have that to look forward to now 😛

What do you think about the idea above?

[hithomasmorelli]

In our templates, all JS files are ignored by default except .eslintrc.js (the condition was added in #2647 - by a familiar author 😉 ).

Arrgh, my own work is coming back to haunt me. Does this mean I've made it?! I hope I don't get a reputation for fixing one bug and creating another 😄

On a serious note, @bkendall what you're suggesting looks good to me. However, this comes with the caveat that I have little experience with tsconfig.json files in particular, so I wouldn't trust myself to comment authoritatively on them! If what you're saying is correct (and I assume it is), then it seems like a good idea.

I agree that, if possible (which it seems to be), ESLint should also be validating .eslintrc.js. In addition, changing npm run lint to eslint . would hopefully avoid any future issues in the same realm as #2644.

I also completely agree with you on trying to keep "all-terminal" and IDE development experiences the same.

On a side note - thank you for your digging! I had done some trying to get to the bottom of this myself, but I ended up between firebase-tools and vscode-eslint. I never would have thought to check for changes between ESLint versions!