Skip to content

Commit 9543dd7

Browse files
authored
chore: Enable a few eslint rules that overlap with biomes rule list (#83693)
Looking at our biome config here we can see all the linty rules that are enabled for JavaScript files: https://github.com/getsentry/sentry/blob/fc9ccb0f4754939a4cf19bfdb08b526f5a491f5a/biome.json#L14-L87 The docs provide the name of the equiv rule inside eslint core, or inside other eslint plugins if available. I mapped the names of the biome rules to those eslint rules here (below). Then I went through each rule and enabled it within the codebase. It was meant to be trivial because biome should've prevented violations, but a few places did need to be fixed up. Anywhere that had a biome-ignore statement needed an eslint one too. There are still a few rules that biome implements which are not in our eslint config right now. The categories are broadly: 1. Rules from plugins that i haven't installed yet, like from `unicorn/*` or from `barrel-files/*`. I'll import these plugins in a followup and check things off the list. 2. Some rules have no eslint equiv: `noApproximativeNumericConstant` & `noMisrefactoredShorthandAssign`, they're also not important to have enabled imo 3. Some rules have an eslint equiv but are also not important: `noShoutyConstants` & `noInvalidUseBeforeDeclaration` would cause lots of churn in the repo for example. Others are repalced by prettier. 4. Some rules depend on type annotations, these are challenging because they are often nice to have but type annotations are _slow_ in eslint. It would make pre-commit checks really terrible if that was enabled. `consistent-type-imports` is an example. I intend to get plugins to fill these specific biome gaps while maintaining pre-commit perf. ---- The rules table is here: | Biome Rule | ESLint Rule | Handled by | | ------------------------------------- | ------------------------------------------------- | ------------------------ | | noBlankTarget | react/jsx-no-target-blank | ✅ react/recommended | noGlobalObjectCalls | no-obj-calls | ✅ tsc | noUnreachable | no-unreachable | ✅ tsc | useIsNan | use-isnan | ✅ eslint/recommended | noUnusedPrivateClassMembers | no-unused-private-class-members | ✅ eslint/recommended | noInvalidUseBeforeDeclaration | @typescript-eslint/no-use-before-define | ✅ Disabled in sentry | noNodejsModules | import/no-nodejs-modules | ✅ eslint.config (except scripts) | useFlatMap | unicorn/prefer-array-flat-map | TODO | useOptionalChain | @typescript-eslint/prefer-optional-chain | TODO typescript/stylistic-type-checked | noEmptyTypeParameters | Prettier? | TODO | noUselessLoneBlockStatements | no-lone-blocks | ✅ eslint.config | noUselessEmptyExport | @typescript-eslint/no-useless-empty-export | ✅ eslint.config | noUselessConstructor | @typescript-eslint/no-useless-constructor | ✅ typescript/strict | noUselessTypeConstraint | @typescript-eslint/no-unnecessary-type-constraint | ✅ typescript/recommended | noExcessiveNestedTestSuites | jest/max-nested-describe (max=5) | ✅ eslint.config | noBarrelFile | barrel-files/avoid-barrel-files | TODO | noDangerouslySetInnerHtmlWithChildren | react/no-danger-with-children | ✅ react/recommended | noDebugger | no-debugger | ✅ eslint/recommended | noDoubleEquals | eqeqeq | ✅ eslint.config | noDuplicateJsxProps | react/jsx-no-duplicate-props | ✅ react/recommended | noDuplicateObjectKeys | no-dupe-keys | ✅ tsc | noDuplicateParameters | no-dupe-args | ✅ tsc | noDuplicateCase | no-duplicate-case | ✅ eslint/recommended | noFallthroughSwitchClause | no-fallthrough | ✅ eslint/recommended | noRedeclare | @typescript-eslint/no-redeclare | ✅ tsc | noSparseArray | no-sparse-arrays | ✅ eslint recommended | noUnsafeDeclarationMerging | @typescript-eslint/no-unsafe-declaration-merging | ✅ typescript/recommended | noUnsafeNegation | no-unsafe-negation | ✅ tsc | useIsArray | unicorn/no-instanceof-array | TODO | noApproximativeNumericConstant | approx_constant | TODO | noMisrefactoredShorthandAssign | misrefactored_assign_op | TODO | useAwait | require-await & @typescript-eslint/require-await | ✅ esint.config | useNamespaceKeyword | @typescript-eslint/prefer-namespace-keyword | ✅ typescript/recommended | noFocusedTests | jest/no-focused-tests | ✅ jest/recommended | noDuplicateTestHooks | jest/no-duplicate-hooks | ✅ eslint.config | noCommaOperator | no-sequences | ✅ eslint.config | noShoutyConstants | ??? | TODO | noParameterProperties | @typescript-eslint/parameter-properties | ✅ typescript | noVar | no-var | ✅ eslint.config | useConst | prefer-const | ✅ tsc | useShorthandFunctionType | @typescript-eslint/prefer-function-type | ✅ typescript/stylistic | useExportType | @typescript-eslint/consistent-type-exports | TODO requires types | useImportType | @typescript-eslint/consistent-type-imports | TODO requires types | useNodejsImportProtocol | unicorn/prefer-node-protocol | TODO | useLiteralEnumMembers | @typescript-eslint/prefer-literal-enum-member | ✅ typescript/strict | useEnumInitializers | @typescript-eslint/prefer-enum-initializers | ✅ | useAsConstAssertion | @typescript-eslint/prefer-as-const | ✅ typescript/recommended | useBlockStatements | curly | ✅ prettier New plugins: - https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-array-flat-map.md - https://github.com/thepassle/eslint-plugin-barrel-files/blob/main/docs/rules/avoid-barrel-files.md - https://tanstack.com/query/v5/docs/eslint/eslint-plugin-query
1 parent fc9ccb0 commit 9543dd7

File tree

10 files changed

+32
-41
lines changed

10 files changed

+32
-41
lines changed

eslint.config.mjs

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ export default typescript.config([
236236
'consistent-return': 'error',
237237
'default-case': 'error',
238238
'dot-notation': 'error',
239+
eqeqeq: 'error',
239240
'guard-for-in': 'off', // TODO(ryan953): Fix violations and enable this rule
240241
'multiline-comment-style': ['error', 'separate-lines'],
241242
'no-alert': 'error',
@@ -268,6 +269,7 @@ export default typescript.config([
268269
'no-sequences': 'error',
269270
'no-throw-literal': 'error',
270271
'object-shorthand': ['error', 'properties'],
272+
radix: 'error',
271273
'require-await': 'error', // Enabled in favor of @typescript-eslint/require-await, which requires type info
272274
'spaced-comment': [
273275
'error',
@@ -277,10 +279,9 @@ export default typescript.config([
277279
block: {exceptions: ['*'], balanced: true},
278280
},
279281
],
282+
strict: 'error',
280283
'vars-on-top': 'off',
281284
'wrap-iife': ['error', 'any'],
282-
radix: 'error',
283-
strict: 'error',
284285
yoda: 'error',
285286

286287
// https://github.com/eslint/eslint/blob/main/packages/js/src/configs/eslint-recommended.js
@@ -369,6 +370,7 @@ export default typescript.config([
369370
{selector: 'typeLike', format: ['PascalCase'], leadingUnderscore: 'allow'},
370371
{selector: 'enumMember', format: ['UPPER_CASE']},
371372
],
373+
372374
'@typescript-eslint/no-restricted-types': [
373375
'error',
374376
{
@@ -392,6 +394,7 @@ export default typescript.config([
392394
],
393395
'@typescript-eslint/no-shadow': 'error',
394396
'@typescript-eslint/no-use-before-define': 'off', // Enabling this will cause a lot of thrash to the git history
397+
'@typescript-eslint/no-useless-empty-export': 'error',
395398
},
396399
},
397400
// https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/base.ts
@@ -406,8 +409,8 @@ export default typescript.config([
406409
// https://typescript-eslint.io/rules/
407410
plugins: {'@typescript-eslint': typescript.plugin},
408411
rules: {
409-
'no-var': 'off', // TODO(ryan953): Fix violations and delete this line
410412
'prefer-spread': 'off', // TODO(ryan953): Fix violations and delete this line
413+
'@typescript-eslint/prefer-enum-initializers': 'error',
411414

412415
// Recommended overrides
413416
'@typescript-eslint/ban-ts-comment': 'off', // TODO(ryan953): Fix violations and delete this line
@@ -427,7 +430,6 @@ export default typescript.config([
427430
'@typescript-eslint/no-extraneous-class': 'off', // TODO(ryan953): Fix violations and delete this line
428431
'@typescript-eslint/no-invalid-void-type': 'off', // TODO(ryan953): Fix violations and delete this line
429432
'@typescript-eslint/no-non-null-assertion': 'off', // TODO(ryan953): Fix violations and delete this line
430-
'@typescript-eslint/prefer-literal-enum-member': 'off', // TODO(ryan953): Fix violations and delete this line
431433
'@typescript-eslint/unified-signatures': 'off', // TODO(ryan953): Fix violations and delete this line
432434

433435
// Stylistic overrides
@@ -439,7 +441,6 @@ export default typescript.config([
439441
'@typescript-eslint/no-empty-function': 'off', // TODO(ryan953): Fix violations and delete this line
440442
'@typescript-eslint/no-inferrable-types': 'off', // TODO(ryan953): Fix violations and delete this line
441443
'@typescript-eslint/prefer-for-of': 'off', // TODO(ryan953): Fix violations and delete this line
442-
'@typescript-eslint/prefer-function-type': 'off', // TODO(ryan953): Fix violations and delete this line
443444

444445
// Customization
445446
'@typescript-eslint/no-unused-vars': [
@@ -558,26 +559,18 @@ export default typescript.config([
558559
// https://github.com/jest-community/eslint-plugin-jest/tree/main/docs/rules
559560
plugins: jest.configs['flat/recommended'].plugins,
560561
rules: {
562+
'jest/max-nested-describe': 'error',
563+
'jest/no-duplicate-hooks': 'error',
564+
'jest/no-large-snapshots': ['error', {maxSize: 2000}], // We don't recommend snapshots, but if there are any keep it small
565+
561566
// https://github.com/jest-community/eslint-plugin-jest/blob/main/src/index.ts
562567
...jest.configs['flat/recommended'].rules,
563568
...jest.configs['flat/style'].rules,
564569

565-
// `recommended` set this to warn, we've upgraded to error
566-
'jest/no-disabled-tests': 'error',
567-
568-
// `recommended` set this to warn, we've downgraded to off
569-
// Disabled as we have many tests which render as simple validations
570-
'jest/expect-expect': 'off',
571-
572-
// Disabled as we have some comment out tests that cannot be
573-
// uncommented due to typescript errors.
570+
'jest/expect-expect': 'off', // Disabled as we have many tests which render as simple validations
574571
'jest/no-commented-out-tests': 'off', // TODO(ryan953): Fix violations then delete this line
575-
576-
// Disabled as we do sometimes have conditional expects
577572
'jest/no-conditional-expect': 'off', // TODO(ryan953): Fix violations then delete this line
578-
579-
// We don't recommend snapshots, but if there are any keep it small
580-
'jest/no-large-snapshots': ['error', {maxSize: 2000}],
573+
'jest/no-disabled-tests': 'error', // `recommended` set this to warn, we've upgraded to error
581574
},
582575
},
583576
{

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@
146146
"qrcode.react": "^3.1.0",
147147
"query-string": "7.0.1",
148148
"react": "18.2.0",
149-
"react-textarea-autosize": "8.5.7",
150149
"react-date-range": "^1.4.0",
151150
"react-dom": "18.2.0",
152151
"react-grid-layout": "^1.3.4",
@@ -156,6 +155,7 @@
156155
"react-router-dom": "^6.26.2",
157156
"react-select": "4.3.1",
158157
"react-sparklines": "1.7.0",
158+
"react-textarea-autosize": "8.5.7",
159159
"react-virtualized": "^9.22.5",
160160
"reflux": "0.4.1",
161161
"screenfull": "^6.0.2",

static/app/components/events/autofix/autofixMessageBox.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,7 @@ function AutofixMessageBox({
481481
}
482482

483483
if (text.trim() !== '' || allowEmptyMessage) {
484-
if (onSend != null) {
484+
if (onSend !== null) {
485485
onSend(text);
486486
} else {
487487
send({

static/app/utils/profiling/renderers/UIFramesRenderer.tsx

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,12 @@ import type {UIFrameNode, UIFrames} from 'sentry/utils/profiling/uiFrames';
66

77
import {upperBound} from '../gl/utils';
88

9-
export interface UIFramesRendererConstructor {
10-
new (
11-
canvas: HTMLCanvasElement,
12-
uiFrames: UIFrames,
13-
theme: FlamegraphTheme,
14-
options?: {draw_border: boolean}
15-
): UIFramesRenderer;
16-
}
9+
export type UIFramesRendererConstructor = new (
10+
canvas: HTMLCanvasElement,
11+
uiFrames: UIFrames,
12+
theme: FlamegraphTheme,
13+
options?: {draw_border: boolean}
14+
) => UIFramesRenderer;
1715

1816
export abstract class UIFramesRenderer {
1917
ctx: CanvasRenderingContext2D | WebGLRenderingContext | null = null;

static/app/utils/profiling/renderers/flamegraphRenderer.tsx

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,12 @@ export const DEFAULT_FLAMEGRAPH_RENDERER_OPTIONS: FlamegraphRendererOptions = {
1717
draw_border: false,
1818
};
1919

20-
export interface FlamegraphRendererConstructor {
21-
new (
22-
canvas: HTMLCanvasElement,
23-
flamegraph: Flamegraph,
24-
theme: FlamegraphTheme,
25-
options?: FlamegraphRendererOptions
26-
): FlamegraphRenderer;
27-
}
20+
export type FlamegraphRendererConstructor = new (
21+
canvas: HTMLCanvasElement,
22+
flamegraph: Flamegraph,
23+
theme: FlamegraphTheme,
24+
options?: FlamegraphRendererOptions
25+
) => FlamegraphRenderer;
2826

2927
export abstract class FlamegraphRenderer {
3028
ctx: CanvasRenderingContext2D | WebGLRenderingContext | null = null;

static/app/utils/statics-setup.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint no-native-reassign:0 */
22

33
// biome-ignore lint/style/noVar: Not required
4-
declare var __webpack_public_path__: string;
4+
declare var __webpack_public_path__: string; // eslint-disable-line no-var
55

66
/**
77
* Set the webpack public path at runtime. This is necessary so that imports

static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ function SetupMessagingIntegrationButton({
6363
messagingIntegrationsQuery.isError ||
6464
integrationProvidersQuery.some(({isPending}) => isPending) ||
6565
integrationProvidersQuery.some(({isError}) => isError) ||
66-
integrationProvidersQuery[0]!.data == null
66+
integrationProvidersQuery[0]!.data === undefined
6767
) {
6868
return null;
6969
}

static/app/views/insights/mobile/screens/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {formatPercentage} from 'sentry/utils/number/formatPercentage';
55
import {VitalState} from 'sentry/views/performance/vitalDetail/utils';
66

77
const formatMetricValue = (metric: MetricValue, field?: string | undefined): string => {
8-
if (metric.value == null) {
8+
if (metric.value === undefined) {
99
return '-';
1010
}
1111
if (typeof metric.value === 'number' && metric.type === 'duration' && metric.unit) {

static/app/views/issueList/utils.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {Organization} from 'sentry/types/organization';
1010
export enum Query {
1111
FOR_REVIEW = 'is:unresolved is:for_review assigned_or_suggested:[me, my_teams, none]',
1212
// biome-ignore lint/style/useLiteralEnumMembers: Disable for maintenance cost.
13-
PRIORITIZED = DEFAULT_QUERY,
13+
PRIORITIZED = DEFAULT_QUERY, // eslint-disable-line @typescript-eslint/prefer-literal-enum-member
1414
UNRESOLVED = 'is:unresolved',
1515
IGNORED = 'is:ignored',
1616
NEW = 'is:new',

tests/js/setup.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,12 @@ declare global {
159159
/**
160160
* Generates a promise that resolves on the next macro-task
161161
*/
162+
// eslint-disable-next-line no-var
162163
var tick: () => Promise<void>;
163164
/**
164165
* Used to mock API requests
165166
*/
167+
// eslint-disable-next-line no-var
166168
var MockApiClient: typeof Client;
167169
}
168170

0 commit comments

Comments
 (0)