Skip to content

Conversation

sawyerh
Copy link
Contributor

@sawyerh sawyerh commented Sep 27, 2022

Changes

These changes fix a number of issues I noticed or ran into when opening the top-level directory in VS Code.

  • Change the ESLint settings file from a JSON file to a JS file, where we now use __dirname for tsconfigRootDir. The previous setting wasn't working for me. This also allows us to add inline code comments to explain the confusing aspects of the file (basically every line).
  • Set root: true so ESLint doesn't attempt to continue searching for lint settings above the app directory
  • Move Jest and TypeScript-specific linter settings into their own overrides blocks. This is an optimization, but also fixed an issue I was running into (see screenshot below)

Testing

Before:

CleanShot 2022-09-26 at 17 27 10@2x

After moving TypeScript lint settings into overrides (working TS and ESLint actions, but redundant errors):

CleanShot 2022-09-26 at 17 28 58@2x

After plugin:@typescript-eslint/eslint-recommended was added to the list (less redundant errors)

CleanShot 2022-09-28 at 18 12 20@2x

@sawyerh sawyerh changed the title Fix tsconfigRootDir + set root Fix ESLint config Sep 27, 2022
@sawyerh sawyerh marked this pull request as ready for review September 27, 2022 00:30
@sawyerh sawyerh requested a review from aligg September 27, 2022 00:30
@sawyerh
Copy link
Contributor Author

sawyerh commented Sep 27, 2022

Moving back to draft. Seeing a couple more issues.

@sawyerh sawyerh removed the request for review from aligg September 27, 2022 00:33
@sawyerh sawyerh marked this pull request as draft September 27, 2022 00:33
@sawyerh sawyerh changed the title Fix ESLint config Fix and optimize ESLint config Sep 29, 2022
@sawyerh sawyerh marked this pull request as ready for review September 29, 2022 01:14
@sawyerh sawyerh requested a review from aligg September 29, 2022 01:15
rules: {
// Prevent dead code accumulation
'@typescript-eslint/no-unused-vars': 'error',
// The usage of `any` defeats the purpose of typescript. Consider using `unknown` type instead instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally unrelated sidebar but for the template did the team discuss the merits of js vs ts somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aligg Good question, I'm not sure. Do you remember @lorenyu @rocketnova?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think so, but at this point I think there's enough in house expertise with typescript and the ecosystem is mature enough that I think it's a pretty easy decision to go with typescript for the type safety which speeds up development

@@ -30,13 +30,14 @@ For linting, this application is leveraging `eslint`, `prettier` and Nava's [esl

In VSCode, do so by creating a `.vscode/settings.json` file with:

```
```json
{
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still means that format on save is done by prettier not eslint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I think this fixAll line results in the --fix flag being ran with ESLint when a file is changed: https://eslint.org/docs/latest/user-guide/command-line-interface#fixing-problems

Copy link
Contributor

@aligg aligg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: still a bit unfamiliar with line by line eslintrc configs but following a read through most make sense /lgtm

@sawyerh sawyerh merged commit 40082da into main Sep 29, 2022
@sawyerh sawyerh deleted the sawyerh/eslint-parser branch September 29, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants