Add eslint

[jaredpalmer]

No description provided.

[diegohaz]

Why not eslint? Even TS is migrating to it: microsoft/TypeScript#30553

[jaredpalmer]

Yeah made issue before the announcement

[bbugh]

I just started using tsdx and found the lack of linting surprising!

I've added linting with @typescript-eslint with prettier integrated, using lint-staged with husky for running on staged files, and it's worked out very well.

Does that sound useful for a patch?

[swyxio]

we have a tsdx lint command that is a bit stale - #99 i havent been involved with the discussion tho and am not sure what @jaredpalmer's plan is here

[bbugh]

@jaredpalmer @skvale #99 was shipped, thank you! 🌟 But there are some issues with it...

1. Even if the "basic" template is selected, the react-app package is added to the .eslintrc.js file when the yarn lint --write-file command is run:

   module.exports = {
     "extends": [
       "react-app",
       "prettier/@typescript-eslint",
       "plugin:prettier/recommended"
     ]
   }

2. Another react issue, when running yarn lint src (or any path), eslint prints:

   Warning: React version was set to "detect" in eslint-plugin-react settings, but the "react" package is not installed. Assuming latest React version for linting.

3. husky is still only running prettier, not the eslint + prettier package. This should call the new lint command. (This would also eliminate the need to include prettier and prettier-quick in the library's own devDependencies)

4. yarn lint without files incorrectly reports success ✨ Done in 2.20s.. When running eslint or prettier without any inputs, it returns the help and in prettier's case, an error code 1. The lint should not fail silently, because that's the same result as a success. If no files are specified, I think it should follow the standard CLI behavior and print the help message.

Thanks for reading!

[jaredpalmer]

Can you submit a PR with these changes?

[bbugh]

Sure thing. I pushed #189 to fix 3 and 4. 1 and 2 require some extra attention.

1. I'm not sure how best to handle this issue, as the lint command doesn't know if the template was basic or react. I was thinking either: 1) changing the write-files flag from boolean to string, so you'd do --write-files basic or --write-files react, or 2) checking package.json for the react dependency. Any preference or other ideas?
2. There's no way to suppress the ESLint React warning - it was requested and rejected. If eslint-plugin-react has detect turned on, it will warn. Is there a silent version of "detect" ? jsx-eslint/eslint-plugin-react#2276 Not sure how to address this, as it seems internal to tsdx. Removing the react line from the project's .eslintrc.js file doesn't disable the eslint react rules.
3. This is fixed by Fix lint command usage by husky and required files #189
4. This is fixed by Fix lint command usage by husky and required files #189

[robphoenix]

In a basic tsdx project, to deal with 1. @bbugh mentioned

Even if the "basic" template is selected, the react-app package is added to the .eslintrc.js file when the yarn lint --write-file command is run:

we have to directly call eslint rather than tsdx lint, in package.json:

"lint": "eslint --fix \"./src/**/*.ts\""

Is there a way to override/ignore the tsdx eslint config? Or document this as a recommended option?