Skip to content

eslint --fix corrupts source and desyncs the suppressions baseline because lint rules conflict with the repo's actual TS setup #1644

Description

@mdonnalley

Summary

The repo's ESLint configuration enables autofixable rules that fight each other and disagree with how the source is actually written and loaded. Running eslint --fix, which the lint-staged pre-commit hook does on every commit, can silently rewrite source into a state that fails tsc and breaks tests, while report-mode yarn lint stays green. The result is that a routine commit touching otherwise-fine files can land broken code.

Impact

  • A commit that stages a handful of source files triggers eslint --fix, which can rewrite those files' imports and types, break the build (tsc) and the test suite, yet still pass the pre-commit hook (the hook runs ESLint, not tsc or tests).
  • yarn lint (report mode) and the hook's eslint --fix (fix mode) disagree, so "lint is green" does not mean "a commit will be safe."
  • Fixing lint findings one file at a time surfaces a new, unrelated failure each time, because the underlying conflicts are config-level, not file-level.

Root cause 1: autofixable rules conflict with the actual source layout, and the baseline hides it

Several rules are set to error with autofixers, but the committed source is written the opposite way and the violations are mass-suppressed in eslint-suppressions.json rather than fixed. Two concrete instances:

  • n/file-extension-in-import: ["error", "always"] requires .js extensions on relative imports, but the source is extension-less TypeScript loaded via ts-node/@oclif/test at test time. The baseline suppresses this rule across ~134 files (~466 occurrences). When eslint --fix touches a file, it "fixes" the import by adding .js, which the test-time loader cannot resolve, so module loading fails and commands/hooks silently produce no output.
  • @typescript-eslint/no-unsafe-return / no-null (and related type-aware rules) get autofixed on individual files (for example rewriting | null return types to | undefined) without touching the corresponding value expressions, producing tsc errors like Type 'null' is not assignable to type 'string | undefined'.

Because these are suppressed globally, they are invisible until an unrelated --fix run converts a suppression into an autofix on one file and desyncs it from the rest of the codebase.

Root cause 2: two import-ordering systems run together and produce circular fixes

Both perfectionist/sort-imports and the eslint-plugin-import (import-x) ordering rules (import-x/first, import-x/newline-after-import, import-x/order) are enabled with autofixers. On files that mix a top-level require with ESM imports (for example src/module-loader.ts, where get-package-type ships no types and is required rather than imported), their fixers disagree on placement and oscillate, emitting ESLintCircularFixesWarning: Circular fixes detected ... likely that you have conflicting rules. The perfectionist/sort-imports docs explicitly recommend disabling the eslint-plugin-import ordering rules to avoid exactly this conflict.

Why the pre-commit flow makes it worse

.lintstagedrc.json runs eslint --fix on staged files. Because the suppressions baseline records pre-fix counts, any --fix that resolves a suppressed violation leaves the baseline stale (There are suppressions left that do not occur anymore), which fails the hook on its own. Working around that (for example adding --prune-suppressions) then lets the other autofixes through, which is how broken .js extensions and type rewrites reach a commit.

Suggested direction (not prescriptive)

  • Reconcile each autofixable rule with how the code is actually written and loaded: either conform the source to the rule, or turn the rule off, rather than mass-suppressing it. In particular decide whether relative imports should carry .js extensions given the ts-node test loader, and set n/file-extension-in-import accordingly.
  • Pick a single import-ordering authority (perfectionist/sort-imports or import-x, not both) per the perfectionist docs.
  • Consider adding tsc --noEmit and/or a test run to the pre-commit or CI gate so autofix-induced breakage cannot pass on a green ESLint alone.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions