Skip to content

Guardrails: typed lint, knip, jscpd and i18n checks in CI - #55

Closed
theobong wants to merge 11 commits into
chore/app-campaign-14-contractfrom
chore/app-campaign-15-guardrails
Closed

theobong wants to merge 11 commits into
chore/app-campaign-14-contractfrom
chore/app-campaign-15-guardrails

Conversation

@theobong

Copy link
Copy Markdown
Member

What changed

The last campaign PR turns the audit's report-only checks into gates that pass on this tree.

  • Typed ESLint in all four workspaces (projectService, source and tests):
    • no-floating-promises and no-misused-promises (JSX attributes on);
    • switch-exhaustiveness-check (a default counts), consistent-type-imports, and every no-unsafe-* rule;
    • every workspace lints with --max-warnings=0.
  • ui: react-hooks/exhaustive-deps is an error. ui, web and mobile run the TanStack Query lint plugin (recommended).
  • Web lint: web moves from next lint and .eslintrc.json to an ESLint 9 flat config run by the ESLint CLI. The host-console import bans are carried over rule for rule.
  • Typechecked tests: mobile tests and the Pages Functions tests are now typechecked.
  • knip (knip.jsonc, pnpm knip): unused exports were un-exported or removed, and unused dev dependencies were dropped. Every remaining ignore has a reason.
  • jscpd (.jscpd.json, pnpm jscpd): the tree measures 0.64% duplication; the threshold is 0.7.
  • i18n:
    • every literal t(...) call site in ui, web and mobile must resolve in its namespace (pnpm i18n:check:call-sites, now part of pnpm i18n:check);
    • the generated catalogs must be fresh (i18n:check:resources).
  • CI: four new steps inside the existing packages (shared + ui) job. No job is renamed or added.

Before you start

  • Stacked on Contract release: @civfix/shared 0.58.0 #54.
  • Where: staging after the main merge (civfix.dev), plus the TestFlight build the merge triggers.
  • Sign in as: a citizen; a host with an event.
  • Size: 2,332 counted lines. Campaign PR; the author approved PRs over the 400-line cap for this cleanup. Most of it is typing fixes and un-exported names.

Verify

Nothing should look or behave differently. The lint fixes wrap event handlers (() => void run()) and add stable hook dependencies, so these checks cover the handlers they touched.

[Web] [Mobile] Handlers that start async work

  1. Sign in with an email code, then sign out. Expect: the same flow as before.
  2. Start a report, take or choose a photo, place the pin and send it. Expect: the same steps and "Pin dropped. We're on it."
  3. In a chat, send a message, react to one and open a photo. Expect: the same behaviour.

[Web] Host console

  1. Open an event's "Check-in" and check someone in; open "Attendees" and use a bulk action. Expect: the same results and toasts.

[Mobile] Launch

  1. Cold-start the app signed in and in Airplane mode. Expect: the same splash and "Can't reach civfix" behaviour.

Regression

Map and sheets: [Mobile]

  1. Drag the home sheet and pan the map. Expect: the same sheet snapping and map behaviour (hook dependencies changed only by adding stable setters and refs).

Findings addressed

  • The brief's PR 15 list: typed rules, exhaustive-deps, the TanStack plugin, the web flat config, knip, jscpd, the i18n check over the apps, catalog freshness, and typechecked mobile tests.
  • The audit's knip findings (817 unused exports, 1,079 unused exported types, 11 unused dev dependencies) are resolved or documented as ignores.
  • APP-PERF-001 (a turbo cache step in CI) is not added. Tests read files across packages, so turbo's per-package hash could replay a stale pass.

Decisions for the reviewer

  • Disables: two lint disables, each with a reason:
    • the ui feed query key rounds near into a feed cell while the request sends the exact point;
    • the web broadcast preview keys on a fingerprint, so edits that change nothing sent do not drop the preview.
  • TanStack allowlist: the plugin's exhaustive-deps allowlist treats the injected api, qc and geo as stable, so query keys do not change.
  • knip ignores (reasons are inline in knip.jsonc):
    • ui .web/.native twins, which bundlers pick by extension;
    • deliberate aliases in the published contract;
    • fonts read by path;
    • expo-contacts and expo-sms, which a separate PR removes. Drop that entry once they are gone.
  • Removed dev dependencies: @typescript-eslint/eslint-plugin and @typescript-eslint/parser (covered by typescript-eslint), and prettier (nothing invoked it) with its config files.
  • Unhandled rejections: two mobile handlers keep a possible unhandled rejection, exactly as before: sign-out in the boot connectivity notice, and the mic permission request in the viewfinder. Both are logged for a fix PR; the void wrapper changes nothing at runtime.

User-visible copy changes

None.

Tests changed

  • In their own test: commits:
    • source-text pins follow the new () => void handlers, the added stable deps and the un-exported names; each pin still checks the same value or shape;
    • test typing for the no-unsafe-* rules: typed mocks, typed JSON.parse and catch, rejection(), which is stricter than .catch(e => e).
  • Barrel pins: two dropped with the barrel names they pinned (useOnboardingTourPresenter, makeFakeOpenInternalHref). The behaviour tests in the same files still cover both.
  • New: fixture tests for the i18n call-site checker (missing key, wrong namespace, unknown namespace, object key, template key).
  • No assertion was loosened, skipped or retimed.

Verification

  • Node v22.23.2, pnpm 9.12.0, in the PR worktree:
    • The three CI jobs' steps ran locally and pass: packages, community-web and community-mobile (including expo-doctor).
    • pnpm install --frozen-lockfile passes, and the committed lockfile equals pnpm's own output.
    • Tests: shared 1352, ui 6561, web 1143, mobile 698, and the root script tests, all passing.
    • The web export's pages and head meta are identical to the previous PR's. The @civfix/shared surface is unchanged.
  • One adversarial review: clean.
    • Its should-fixes are done:
      • the test edits are in test: commits;
      • the README lists the new scripts;
      • the knip ignore comment no longer points at a campaign artifact;
      • the leftover prettier config is deleted;
      • the checker has fixture tests.
    • One nit is left as is: the TanStack allowlist keys on the geo variable, because the plugin's type allowlist does not match that interface.
  • CI runs only on pull requests into main, so it starts when this PR retargets after Contract release: @civfix/shared 0.58.0 #54 merges.

Not covered

  • A clean-runner CI run: it happens on retarget.

🤖 Generated with Claude Code

@byteful byteful closed this Sep 24, 2026
@byteful
byteful deleted the chore/app-campaign-15-guardrails branch September 24, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants