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.

Sign up for GitHub

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

Update typescript-eslint monorepo to v7 (major) #518

Merged
merged 20 commits into from
Apr 19, 2024
Merged

Update typescript-eslint monorepo to v7 (major) #518

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fadb7d6
Update typescript-eslint monorepo to v7
renovate[bot] Apr 17, 2024
bcb735b
update eslint-config-airbnb-typescript so that upgrading typescript-e…
philfuster Apr 18, 2024
a674449
update deprecated rule name.
philfuster Apr 18, 2024
30b1e97
New naming selector for 'import', triggering errors because it fell t…
philfuster Apr 18, 2024
3143131
remove union with null for return type. eslint was saying unknown tru…
philfuster Apr 18, 2024
dbfe610
remove unused no-non-null-assertion override eslint-disable directive.
philfuster Apr 18, 2024
d38589f
Revert "remove unused no-non-null-assertion override eslint-disable d…
philfuster Apr 18, 2024
cfc8127
re-add no-non-null-assertion rules. It was removed from defaults.
philfuster Apr 18, 2024
2ea2e56
move eslint disable down to relevant line.
philfuster Apr 18, 2024
4596d01
enforce consistent index object style.
philfuster Apr 18, 2024
94fa0bd
turn off consistent-indexed-object-style. We use both as necessary. I…
philfuster Apr 18, 2024
7606e55
change to use function type. eslint was complaining and I don't see w…
philfuster Apr 18, 2024
70d5c1c
turn off consistent indexed object style because both styles are used…
philfuster Apr 18, 2024
8e13a5d
change to use optional chain expression because eslint rule and becau…
philfuster Apr 18, 2024
00be138
override prefer-nullish-coalescing because it includes scenarios where
philfuster Apr 18, 2024
acf2b18
no longer turning off consistent-indexed-object-style. Global consist…
philfuster Apr 18, 2024
338a212
move prefer-nullish-coalescing rule to jsTsGlob.
philfuster Apr 18, 2024
39c20bb
Merge branch 'main' into renovate/major-typescript-eslint-monorepo
philfuster Apr 18, 2024
b468608
run install and update to make sure package-lock.json is up-to-date.
philfuster Apr 18, 2024
8a1755a
move prefer-nullish-coalescing rule back to tsGlob block.
philfuster Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ module.exports = {
},
// camelCase for everything not otherwise indicated
{ selector: 'default', format: ['camelCase'] },
// allow any naming convention for imports
{ selector: 'import', format: null },
// allow known default exclusions
{ selector: 'default', filter: { regex: '^(_id|__v|_raw)$', match: true }, format: null },
// allow variables to be camelCase or UPPER_CASE
Expand Down Expand Up @@ -72,6 +74,7 @@ module.exports = {
'plugin:jest/recommended',
'plugin:jest/style',
'plugin:@typescript-eslint/recommended',
'plugin:@typescript-eslint/stylistic',
'plugin:import/typescript',
'prettier',
],
Expand Down Expand Up @@ -139,11 +142,14 @@ module.exports = {
],
'@typescript-eslint/consistent-type-imports': ['error', { prefer: 'type-imports' }],

// disable consistent indexed object style. both are used in mvom
Copy link
Contributor

Choose a reason for hiding this comment

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

We use Record and the index signature. The Record is good in most scenarios as used in mvom but,
TypeScript will report errors about recursive type reference in scenarios like in Schema.ts where SchemaDefinition references SchemaTypeDefinition and SchemaTypeDefinition references SchemaDefinition.

I think turning the rule off is better than trying to force one or the other. I think I liked the use of Record where it made sense and the use the index signature where needed as well.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to see a global enforcement of consistency and an override of the rule where it's not possible to use that. It looks like its by design that Record cannot circularly reference itself: microsoft/TypeScript#33050 (comment)

However, for those scenarios where that's not the case, it would be better to have consistency and enforce the rule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed the override to the turn the rule off. I updated where possible to use Record and provided an override for where not possible.

'@typescript-eslint/consistent-indexed-object-style': 'off',

// enforce consistent order of class members
'@typescript-eslint/member-ordering': 'error',

// disallow parameter properties in favor of explicit class declarations
'@typescript-eslint/no-parameter-properties': 'error',
'@typescript-eslint/parameter-properties': 'error',

// ensure unused variables are treated as an error
// overrides @typescript-eslint/recommended -- '@typescript-eslint/no-unused-vars': 'warn'
Expand All @@ -153,7 +159,10 @@ module.exports = {
},
{
files: tsGlobs,
extends: ['plugin:@typescript-eslint/recommended-requiring-type-checking'],
extends: [
'plugin:@typescript-eslint/recommended-type-checked',
'plugin:@typescript-eslint/stylistic-type-checked',
],
rules: {
// ban ts-comment except with description
'@typescript-eslint/ban-ts-comment': [
Expand All @@ -166,8 +175,8 @@ module.exports = {
},
],

// disable rules turned on by @typescript-eslint/recommended-requiring-type-checking which are too noisy
// https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.ts
// disable rules turned on by @typescript-eslint/recommended-type-checked which are too noisy
// https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/recommended-type-checked.ts
'@typescript-eslint/no-unsafe-argument': 'off',
'@typescript-eslint/no-unsafe-assignment': 'off',
'@typescript-eslint/no-unsafe-call': 'off',
Expand All @@ -176,9 +185,22 @@ module.exports = {
'@typescript-eslint/restrict-template-expressions': 'off',
'@typescript-eslint/unbound-method': 'off',

// override @typescript-eslint/stylistic-type-checked to ignore consistent-indexed-object-style. Both are used as required.
'@typescript-eslint/consistent-indexed-object-style': 'off',
Copy link
Member

Choose a reason for hiding this comment

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

If we did need to disable this rule, we shouldn't have to disable it in both the jsTsGlobs override and the tsGlobs override. The jsTsGlobs override should have covered both.

But as noted previously, I think we should enable the rule.

Copy link
Contributor

Choose a reason for hiding this comment

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

I moved this to jsTsGlobs I understood your comment as still wanting the override. if that's not the case, I will remove it.

Copy link
Member

Choose a reason for hiding this comment

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

My comment was on @typescript-eslint/consistent-indexed-object-style which you turned off in both the jsTsGlobs and the tsGlobs. Since tsGlobs is a subset of jsTsGlobs you effectively turned the rule off twice. Since you were going to change this anyway, it was to provide an understanding that you were doing something redundant.

However, it seems like you've moved the prefer-nullish-coalescing rule to jsTsGlobs, but that wasn't the one I commented on. That rule requires type checking, which isn't enabled for jsTsGlobs. It has to be moved back.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh ok gotcha. Thank you. I moved prefer-nullish-coalescing back to tsGlobs.


// force explicit member accessibility modifiers
'@typescript-eslint/explicit-member-accessibility': 'error',

// override @typescript-eslint/stylistic-type-checked to ignore booleans in nullish coalescing checks
// https://typescript-eslint.io/rules/prefer-nullish-coalescing#ignoreprimitives
'@typescript-eslint/prefer-nullish-coalescing': [
'error',
{ ignorePrimitives: { boolean: true } },
],

// ban non-null assertions
'@typescript-eslint/no-non-null-assertion': 'error',
Copy link
Member

Choose a reason for hiding this comment

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

I usually try to group related rules or keep them at least alphabetically ordered. So I would flip-flop these two for readability.


// disallow boolean comparisons against non-boolean values
'@typescript-eslint/strict-boolean-expressions': ['error', { allowNullableBoolean: true }],
},
Expand Down
Loading