Skip to content

CI never runs tsc --noEmit — 9 type errors live undetected in test files because next build does not type-check them #425

Description

@Jagadeeshftw

Priority: High  ·  Area: CI / type safety  ·  Est. effort: 6–10 h

📌 Problem

.github/workflows/ci.yml runs npm ci, npm run lint, npm run build, npm run test. That is a real pipeline — but it has a specific blind spot.

npm run build is next build, which type-checks only the modules reachable from the application's build graph. Test files are not in that graph. package.json defines no typecheck script, so tsc --noEmit never runs against the whole project.

The result: npx tsc --noEmit reports 9 errors, all in test files, and CI is green.

src/components/SettlementTable.test.tsx   5 errors
src/components/MetricsBar.test.tsx        4 errors

Both are TS2345 — arguments not assignable to the expected parameter type. These are not cosmetic: they mean the tests are passing values that do not match the real contracts they claim to exercise, which is tracked as its own issue for each file.

🎯 Design decision required

State and defend:

  1. Fix-then-gate, or gate-then-fix? With only 9 errors in 2 files, clearing them and adding the gate in one PR is realistic. If you disagree, argue for the ratchet.
  2. Scope of the gate. Should tsc --noEmit cover test files, or should tests be excluded from the project's type-checking? Excluding them is a real option some projects choose — but argue it honestly, because these 9 errors are exactly the kind of defect exclusion would hide.
  3. Relationship to next build. Should the gate be a separate typecheck step, or is a tsconfig change enough to make next build cover everything? Say which and why.

🧩 Requirements and context

  • Fixing the 9 errors properly is in scope. They point at real mismatches between test doubles and production contracts. MetricsBar.test.tsx and SettlementTable.test.tsx each have a dedicated issue in this repo describing the specific defect — coordinate, and if those land first, this issue is just the gate.
  • Do not add @ts-ignore/@ts-expect-error or a tsconfig exclusion to reach zero. That would hide precisely what the gate exists to reveal.
  • Add a typecheck script and wire it into CI as a step distinct from build, so a type failure is distinguishable from a bundling failure.
  • All 59 test files must still pass.

🛠️ Suggested execution

  1. Run npx tsc --noEmit and paste all 9 errors.
  2. Fix them, or confirm the per-file issues have landed.
  3. Add the typecheck script and the CI step.
  4. Prove it fires — introduce a deliberate type error in a test file on a scratch branch, show the failing run, revert.
  5. Confirm the full suite passes.

✅ Acceptance criteria

  • npx tsc --noEmit exits 0.
  • A typecheck script exists and runs in CI as a step separate from build.
  • The gate is proven to fail on a type error inside a test file — the case next build misses (link the run).
  • No @ts-ignore, @ts-expect-error or tsconfig exclusion was added.
  • All 59 test files pass.
  • The PR states the scope decision for test files.

🚫 Out of scope

  • Coverage enforcement — separate issue.
  • Adding accessibility or e2e tests — separate issues.
  • Refactoring components.

🧪 Verification

npm ci
npx tsc --noEmit
npm run lint
npm run build
npm test

📤 What your PR must include

  1. The 9 errors, before.
  2. Your fix-vs-gate sequencing and test-file scope decisions.
  3. A link to a run failing on a test-file type error.
  4. Confirmation the full suite passes.
  5. Closes #<n>.

🔒 Security notes

The gap is not the nine errors — it is that a whole category of code is outside type-checking while CI reports green. Test files are where mocks assert what production contracts look like; a type error there means the tests and the application disagree about a shape, and the tests still pass. That is how a component can be thoroughly "tested" against an interface it never actually receives.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

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

Metadata

Metadata

Assignees

Labels

GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuepriority:highHigh difficulty / architectural or cross-cutting

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions