eslint disabled for static-site

jameshadfield commented 5 months ago

Current Behavior

Currently linting is (seemingly) disabled for the static-site contents. Running with --debug indicates it is inheriting the static-site/ ignore pattern from the parent directory. Removing the line

https://github.com/nextstrain/nextstrain.org/blob/c3dd7262f8cff0a5093567629cc4d55d55668ad6/.eslintrc.yaml#L17

allows npx eslint --ext .js,.jsx . to work as expected within the static-site directory and (but?) also flags up linting errors in the static-site when run from the root directory.

The static site has a .eslintrc.yaml which indicates that our intention is to lint it (but using our own rules):

https://github.com/nextstrain/nextstrain.org/blob/c3dd7262f8cff0a5093567629cc4d55d55668ad6/static-site/.eslintrc.yaml#L1-L2

@victorlin I think you're best placed here at the moment -- is this expected behaviour? I would think we want to be linting the static-site. It currently has around 30k (yes 30,000) linting errors.

Eslint versions

This is an aside, but I'll mention it her so others don't have to investigate.

From the root directory we have eslint v8.23.1 (via npm ci), however within the static site we have version 6.8.0 (also via npm ci). The static-site's eslint is via:

gatsby (has a dependency of) eslint-config-react-app (has a peer dep of) @typescript-eslint/eslint-plugin (has a peer dependency of) eslint "^5.0.0 || ^6.0.0"

This doesn't matter, as running ../node_modules/.bin/eslint --ext .js,.jsx . from the static-site gives the same output as below (but with the reported eslint version being v8).

Expected behavior

Static site JS is linted.

How to reproduce

Steps to reproduce the current behavior:

cd static-site
npm ci
npx eslint --ext .js,.jsx .
Error:
```
Oops! Something went wrong! :(
```

ESLint: 6.8.0.

You are linting ".", but all of the files matching the glob pattern "." are ignored.

If you don't want to lint these files, remove the pattern "." from the list of arguments passed to ESLint.

If you do want to lint these files, try the following solutions:

Check your .eslintignore file, or the eslintIgnore property in package.json, to ensure that the files are not configured to be ignored.
Explicitly list the files from this glob that you'd like to lint on the command-line, rather than providing a glob as an argument.

jameshadfield commented 5 months ago

Once eslint is working for the static-site, we should use settings like the following in ./static-site/.eslintrc.yaml:

extends:
  - plugin:react/recommended
  - plugin:react-hooks/recommended

settings:
  react:
    version: "16"

ignorePatterns:
  - public

Which reduce the eslint errors down to ~400. To get this working in vs code I had to install those plugins in the root directory (!) and update the root .eslintrc.yaml to include

  babelOptions:
    plugins: ["@babel/plugin-proposal-class-properties"]
    presets: ["@babel/preset-react"]

I'm sure there's a better way, but I wanted to focus on building things so didn't spend time debugging this further.

victorlin commented 5 months ago

Sorry about this! I excluded static-site from ESLint in e300df0a72a11116cdca75443a97cd557715f6b2 but looking back now, I don't see a good reason. I might've realized that the same ESLint rules were being applied to multiple projects, which is not good, but instead of addressing all projects, only the top-level project was addressed. This means the same goes to the auspice-client project.

I'll look into restoring ESLint on these directories.

jameshadfield commented 5 months ago

Sorry about this!

No problem at all - and I wasn't attempting to assign this to you, rather to check if my experience was expected or not. I was able to either get it working via command line or in VS-code, but ideally they'd both work. Separating the static-site deps and rules from the server code makes sense to me, but the interplay of different configs, different eslint versions etc was confusing to me when I dove in.

victorlin commented 5 months ago

Due to our monorepo structure, we have to be explicit to make sure that each project's sources are independent, including source linting. In the PR linked above, the root: true ESLint configuration disables ESLint's default behavior of configuration cascading.

nextstrain / nextstrain.org