Skip to content

Enable ESLint in CI (flat-config migration) — split out of #135 #141

Description

@NathanTarbert

Summary

Enable ESLint in CI. This was originally part of #135 and was split out after four CR
rounds showed that most findings were edges of this one change — it alters lint resolution for
every package, for editors, for per-package invocation, and for apps/web's separate Next
config. It needs to land where those effects are the only thing under review.

Why it is not already on

CI's required check is named "Lint, Typecheck & Test" but runs only build, typecheck and
test. #135 adds a comment making that gap explicit rather than silently misleading, but does
not close it.

The blocker: ESLint 9 defaults to flat config, this repo uses .eslintrc.cjs, and the
ESLINT_USE_FLAT_CONFIG=false opt-out does not survive turbo's env sanitization.

Work, in the order it was validated during the split

  1. eslint.config.cjs translating the current rules via FlatCompat (@eslint/eslintrc
    is already installed, but must be declared — it and @eslint/js are otherwise phantom
    deps resolving only through pnpm's hoisting of eslint's transitives).
  2. Exclude apps/web (ignores: ['apps/web/**']). It keeps its own .eslintrc.cjs with
    the Next and react-hooks plugins and is linted by next lint. Without the exclusion, the
    root flat config shadows it and npx eslint there fails with
    Definition for rule 'react-hooks/exhaustive-deps' was not found.
  3. Add root: true to apps/web/.eslintrc.cjs. The root eslintrc currently provides it;
    once that file is deleted, web's config cascades past the repo into the developer's
    ~/.eslintrc* and lint results vary by machine.
  4. Fix the 4 errors this surfaces — three empty-interface and one no-explicit-any, all
    already fixed in chore: secret hygiene, deployment doc corrections, and typing fixes #135 — plus apps/worker's lint script, currently
    echo 'no eslint config for worker yet', which exempts the service that owns the
    SHADOW_MODE post-back gate.
  5. turbo.json: declare the config as a globalDependencies cache input (in chore: secret hygiene, deployment doc corrections, and typing fixes #135), so a
    rule change does not serve a cached pass.
  6. Add the CI Lint step and confirm the required check finally does what its name says.

Decisions worth making deliberately

  • Warnings are unbounded. ~48 exist repo-wide (no-unused-vars,
    consistent-type-imports, plus react-hooks/exhaustive-deps and no-img-element in web).
    Adding --max-warnings 0 means clearing them first; without it the gate blocks on errors
    only. Either is defensible — decide, and say which in the PR.
  • no-unused-vars sets only argsIgnorePattern: '^_', so the repo's own _-prefix
    convention still warns on variables (_transformed, _ticket). Adding varsIgnorePattern
    and caughtErrors would remove several warnings for free.
  • Coverage gaps to name explicitly: scripts/*.ts and the root vitest.config.ts sit
    outside every workspace package so turbo run lint never reaches them (0 errors, 4 warnings
    when linted directly); apps/docs has no lint script at all;
    apps/discord-mcp/test/server.test.ts and the per-package vitest.config.ts files are
    linted by nothing.

Verification the split proved necessary

Comparing problem counts in one package is not sufficient — that is what let the
apps/web rule-shadowing through. Verify every affected scope: a package using the root
config, apps/web (both bare eslint and next lint), a per-package invocation from inside
the package directory, and an editor/LSP run.

Acceptance criteria

  • pnpm lint passes from the repo root with no environment variable set
  • cd apps/<any> && pnpm lint passes standalone
  • apps/web keeps its Next + react-hooks rules under next lint, and bare eslint there
    does not error
  • Lint runs in CI and the required check name matches what it does
  • apps/worker is no longer exempt

Split out of #135.

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

    area: infrastructureWorker, queue, CI, deploy, containers, observabilityroadmapTracked on the Outpost roadmaproadmap: nextRoadmap horizon: after launch path clears

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions