-
Notifications
You must be signed in to change notification settings - Fork 231
feat(eslint-config): enable linting with type information #1098
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
feat(eslint-config): enable linting with type information #1098
Conversation
🦋 Changeset detectedLatest commit: be4c434 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1098 +/- ##
=======================================
Coverage 79.50% 79.50%
=======================================
Files 77 77
Lines 2279 2279
Branches 577 577
=======================================
Hits 1812 1812
Misses 391 391
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
languageOptions: { | ||
parserOptions: { | ||
projectService: true, | ||
tsconfigRootDir: import.meta.dirname, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tells the typescript-eslint
parser where to find the root tsconfig.json
, and to use project references
linterOptions: { | ||
reportUnusedDisableDirectives: 'error', | ||
reportUnusedInlineConfigs: 'error', | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional linter options
reportUnusedDisableDirectives
defaults to 'warn'
and reportUnusedInlineConfigs
defaults to 'off'
rules: { | ||
'@typescript-eslint/array-type': 'off', | ||
'@typescript-eslint/await-thenable': 'off', | ||
'@typescript-eslint/consistent-indexed-object-style': 'off', | ||
'@typescript-eslint/consistent-type-definitions': 'off', | ||
'@typescript-eslint/dot-notation': 'off', | ||
'@typescript-eslint/no-base-to-string': 'off', | ||
'@typescript-eslint/no-confusing-void-expression': 'off', | ||
'@typescript-eslint/no-deprecated': 'off', | ||
'@typescript-eslint/no-duplicate-type-constituents': 'off', | ||
'@typescript-eslint/no-dynamic-delete': 'off', | ||
'@typescript-eslint/no-floating-promises': 'off', | ||
'@typescript-eslint/no-invalid-void-type': 'off', | ||
'@typescript-eslint/no-misused-promises': 'off', | ||
'@typescript-eslint/no-non-null-assertion': 'off', | ||
'@typescript-eslint/no-redundant-type-constituents': 'off', | ||
'@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', | ||
'@typescript-eslint/no-unnecessary-condition': 'off', | ||
'@typescript-eslint/no-unnecessary-template-expression': 'off', | ||
'@typescript-eslint/no-unnecessary-type-arguments': 'off', | ||
'@typescript-eslint/no-unnecessary-type-assertion': 'off', | ||
'@typescript-eslint/no-unnecessary-type-parameters': 'off', | ||
'@typescript-eslint/no-unsafe-argument': 'off', | ||
'@typescript-eslint/no-unsafe-assignment': 'off', | ||
'@typescript-eslint/no-unsafe-call': 'off', | ||
'@typescript-eslint/no-unsafe-enum-comparison': 'off', | ||
'@typescript-eslint/no-unsafe-member-access': 'off', | ||
'@typescript-eslint/no-unsafe-return': 'off', | ||
'@typescript-eslint/no-useless-constructor': 'off', | ||
'@typescript-eslint/non-nullable-type-assertion-style': 'off', | ||
'@typescript-eslint/only-throw-error': 'off', | ||
'@typescript-eslint/prefer-function-type': 'off', | ||
'@typescript-eslint/prefer-includes': 'off', | ||
'@typescript-eslint/prefer-nullish-coalescing': 'off', | ||
'@typescript-eslint/prefer-optional-chain': 'off', | ||
'@typescript-eslint/prefer-regexp-exec': 'off', | ||
'@typescript-eslint/prefer-return-this-type': 'off', | ||
'@typescript-eslint/require-await': 'off', | ||
'@typescript-eslint/restrict-plus-operands': 'off', | ||
'@typescript-eslint/restrict-template-expressions': 'off', | ||
'@typescript-eslint/unbound-method': 'off', | ||
'@typescript-eslint/unified-signatures': 'off', | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've initially disabled all the new rules to keep this PR small
I'd be happy to revisit each rule individually and decide if it's worth enabling or not. If a new rule is problematic, it can always be disabled in the shared eslint config
My personal preference is to have less configuration, and rely on tooling to make these decisions for me. But I understand that doesn't fit with how everyone works 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've initially disabled all the new rules to keep this PR small
I like this approach. Fixing lint errors is troublesome, and the diffs will be huge. We don't have to do it.
...tseslint.configs.strictTypeChecked, | ||
...tseslint.configs.stylisticTypeChecked, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few shared configs to choose from.
Their suggestion is to enabling recommended-type-checked
and stylistic-type-checked
to start with. Then replacing recommended-type-checked
with strict-type-checked
if you're comfortable working with TypeScript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Includes bun_test
files in the project
tsconfig.repo-config-files.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Includes project configuration files
50e796c
to
eb62c87
Compare
…eslint-type-checked
@@ -0,0 +1,10 @@ | |||
--- | |||
'@hono/eslint-config': major |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This change includes breaking change, so this is a major
version up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to ping me if you need help upgrading other projects that use @hono/eslint-config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Looks good! For this PR, let's just update |
Updates the
@hono/eslint-config
to includetypescript-eslint
presets for linting with type information