Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use kouts/eslint-config for linting #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

kouts
Copy link

@kouts kouts commented Oct 29, 2024

Thanks for creating this Express template, it looks great!
This PR replaces the ESLint config with https://github.com/kouts/eslint-config which uses https://github.com/neostandard/neostandard under the hood.

Copy link
Author

Choose a reason for hiding this comment

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

This file is no longer needed

rules: {
"@typescript-eslint/consistent-type-imports": "error",
"@typescript-eslint/consistent-type-exports": "error"
'@typescript-eslint/consistent-type-definitions': 'off',
Copy link
Author

Choose a reason for hiding this comment

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

It's a good practice to use types over interfaces, but since you don't have any specific preference I've turned this rule off.
Learn more about it here:
https://www.totaltypescript.com/type-vs-interface-which-should-you-use
https://www.youtube.com/watch?v=zM9UPcIyyhQ

Copy link
Owner

Choose a reason for hiding this comment

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

I generally prefer using types over interfaces, except in cases where I want to create a "contract" that certain classes must implement. In those situations, I find interfaces to be more appropriate.

What are your thoughts on the consistent-type-imports and consistent-type-exports rules? Do you think they provide value or might they not be worth the effort?

Copy link
Author

@kouts kouts Nov 2, 2024

Choose a reason for hiding this comment

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

Actually by default @kouts/eslint-config has

'@typescript-eslint/consistent-type-imports': [
    'error',
    {
      prefer: 'type-imports',
      fixStyle: 'inline-type-imports',
    },
  ],

so that it enforces a consistent style for importing types in TypeScript files, specifically focusing on distinguishing between importing types and importing values.
This can be overridden though in case you don't like it.

Comment on lines 21 to -47
"devDependencies": {
"@eslint/js": "^9.9.1",
"@kouts/eslint-config": "^1.3.17",
"@tsconfig/node20": "^20.1.4",
"@types/cors": "^2.8.17",
"@types/eslint__js": "^8.42.3",
"@types/eslint-config-prettier": "^6.11.3",
"@types/express": "^4.17.21",
"@types/jest": "^29.5.12",
"@types/morgan": "^1.9.9",
"@types/node": "^20.16.5",
"@types/supertest": "^6.0.2",
"@typescript-eslint/eslint-plugin": "^8.3.0",
"@typescript-eslint/parser": "^8.3.0",
"eslint": "^9.9.1",
"eslint-config-prettier": "^9.1.0",
"globals": "^15.9.0",
"husky": "^9.1.5",
"jest": "^29.7.0",
"lint-staged": "^15.2.9",
"madge": "^8.0.0",
"prettier": "^3.3.3",
"prettier-plugin-organize-imports": "^4.1.0",
"supertest": "^7.0.0",
"ts-jest": "^29.2.5",
"tslib": "^2.7.0",
"typescript": "^5.6.2",
"typescript-eslint": "^8.5.0"
Copy link
Author

Choose a reason for hiding this comment

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

We can remove many packages and declutter package.json since all of this is handled by @kouts/eslint-config

@@ -0,0 +1,3 @@
import prettierConfig from '@kouts/eslint-config/prettier'

export default { ...prettierConfig, semi: true }
Copy link
Author

Choose a reason for hiding this comment

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

I've set this to semi: true here as that was how it was set before but I've recently switched to not using semis and I think it's actually better.
This is the article that changed my mind: https://www.epicweb.dev/your-code-style-does-matter-actually

Copy link
Owner

Choose a reason for hiding this comment

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

I'm used to working without semicolons in Go, but I think they are more common and expected in JS/TS projects.

Copy link
Author

Choose a reason for hiding this comment

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

Yep agree that it's more common to use them than not, that's why I've set semi: true 😄

@@ -6,6 +6,7 @@ import { logger } from './logger';
import { router } from './routes';

const app = express();

Copy link
Author

Choose a reason for hiding this comment

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

These empty lines here are set by the https://eslint.org/docs/latest/rules/padding-line-between-statements which I think helps a lot with readability.

@@ -1,4 +1,4 @@
import winston from 'winston';
import type winston from 'winston';
Copy link
Author

Choose a reason for hiding this comment

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

Also a good practice to explicitly state that this is a type you are importing (enforced by an ESLint rule as well)

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.

2 participants