Repository navigation
[DevEx]: Type safety — editor diagnostics and pre-merge CI type checking #1399
Description
Improvement Summary
Turn on JS type checking so bad enum keys and stale property access show as red lines in the editor and fail CI before merge. Measured, staged, and tracked by the child issues below.
Detailed Description
#1393 / #1394 / #1397 made the shared JSON enums resolve in the editor. They do not make anything report — checkJs is off, so you get autocomplete but no diagnostics:
checkJs: true -> error TS2339: Property 'TOTALLY_FAKE_KEY' does not exist on type
'{ CREATED: number; STARTED: number; FINISHED: number; ARCHIVED: number; }'
checkJs: false -> (no errors reported)
This is not theoretical. Enabling checkJs for an afternoon surfaced three real, shipped bugs in one file — the blank game-log row from #1386, plus two glasses-eight messages that could never fire (both fixed in #1398). Nothing in lint, unit tests, or e2e caught any of them.
Measured baseline
Everything below was measured on main, not estimated. tsc for the backend (CommonJS), vue-tsc for the frontend (tsc cannot parse SFCs at all).
| Scope | checkJs today |
+ globals decl | + #1397 merged | Floor |
|---|---|---|---|---|
Backend (api/, utils/, types/) |
262 | 27 | 13 | 0 — reachable |
Frontend (src/, incl. .vue) |
420 | — | — | ~29 |
Backend — 93% is one missing declaration file. Of 262 errors, 243 are undeclared Sails globals: sails (120), Game (35), _ (33), User (21), Match (12), UserSpectatingGame (7), Season (5), Identity (4), user (3), Card (3), GameStateRow (2), userService (1). A ~15-line globals.d.ts takes 262 → 27. Of those 27, 14 are exactly the extensionless imports #1397 already fixes, leaving 13: 8 malformed JSDoc comments and 5 environment gaps. The backend can reach zero in a couple of small PRs.
Frontend — 93% is the legacy Options API. Of 420 errors:
| files | errors | |
|---|---|---|
| Options API components | 29 | 391 |
<script setup> components |
9 | 22 |
plain .js |
5 | 7 |
Two systemic causes account for 306 of the 391: mapStores / this.<x>Store property access (223) and Options API computed circularity, TS2615 (83). <script setup> components are already near-clean.
Wrapping a component in defineComponent() does not fix this — tested on MoveChoiceOverlay.vue, errors stayed at 10. The working fix is migration to <script setup>, which AGENTS.md already mandates for new components, so this converges with an existing standard rather than inventing one.
Concentration is extreme: the top 8 files are 81% of all frontend errors, and GameView.vue alone is 137 (33%).
Strategy
Gate by scope, not all at once. The backend's path to zero is short and certain; the frontend's runs through a 29-component migration. Two tsconfigs are needed regardless — the backend is CommonJS, the frontend is ESM and needs vue-tsc — so splitting the gate costs nothing extra and lands real CI signal immediately instead of after the whole migration.
CI wiring is trivial: .github/workflows/run-npm-script.yml is already a reusable job, so each gate is ~5 lines in pr-actions.yml.
Dependencies
This needs typescript and vue-tsc as devDependencies. Neither is currently in package-lock.json. Per AGENTS.md that requires explicit approval — it is called out in the first child issue rather than assumed.
Both are build-time only and ship nothing to users. checkJs changes no runtime behavior, no bundle output, and no Vite resolution.
Child issues
Ordered; each is independently mergeable. Frontend targets TypeScript, backend stays JS + checkJs — see "Why TS on the frontend" below.
Tooling
- [DevEx]: Add TypeScript tooling, ESLint TS parser, typecheck scripts, and CI jobs #1400 —
typescript+vue-tsc+typescript-eslint, thetypecheck:*scripts, and both CI jobs (needs dependency approval) ← hard blocker on every TS issue below
Warning
Nothing can adopt TypeScript until #1400 lands. npm run lint is a blocking CI job and the repo has no TS-aware ESLint parser, so all TS syntax currently fails to parse — in .vue and plain .ts alike:
const x: string = 'a'; -> Parsing error: Unexpected token :
defineProps<{ n: number }>(); -> Parsing error: Unexpected token )
This already cost a round trip on #1415. The fix is verified ([email protected] + parser wiring) and is part of #1400.
Backend to zero, then gate
-
fix(devex): resolve backend JSON enum imports #1397 — Backend JSON enum imports(merged) - [DevEx]: Declare the Sails globals for type checking (262 → 27 errors) #1401 — Declare the Sails globals (262 → 27) (good first issue)
- [DevEx]: Fix four malformed JSDoc blocks in StatsController and convert-card-to-str #1411 — Fix four malformed JSDoc blocks (8 errors) (good first issue)
- [DevEx]: Close the last 5 backend environment type gaps #1402 — Close the last 5 backend environment gaps (→ 0)
- [DevEx]: Enable checkJs for the backend #1404 — Enable
checkJsfor the backend ← first red squiggles on.js
Frontend — declarations and small type fixes
- [DevEx]: Frontend environment and global type declarations #1403 — Frontend environment and global declarations (~7 errors) (good first issue)
- [DevEx]: Add TS prop types to nine already-migrated script setup components #1428 — TS prop types for nine already-migrated components (9 errors) (good first issue)
- [DevEx]: Triage the remaining frontend type errors in script-setup and plain JS files #1410 — Remaining
<script setup>and plain-JS errors (~22 errors)
Frontend — Options API → <script setup lang="ts">
- [DevEx]: Migrate GameView.vue to script setup (batch 1 — 137 errors, 33%) #1405 — batch 1:
GameView.vue(137 errors, 134 migration-fixable) - [DevEx]: Migrate game dialogs to script setup (batch 2 — 119 errors, → 61%) #1406 — batch 2: game dialogs (119 errors, 118 migration-fixable)
- [DevEx]: Migrate overlays, rules and stats to script setup (batch 3 — 85 errors, → 81%) #1407 — batch 3: overlays, rules, stats (85 errors, 72 migration-fixable)
- [DevEx]: Migrate FourDialog and SpectatorListMenu to script setup + TS #1412 —
FourDialog+SpectatorListMenu(4 of the original 6 merged via refactor: migrate AwardCard to script setup #1416–refactor: migrate StatsLeaderboardCell to script setup #1419) - [DevEx]: Migrate remaining Options API components to script setup (batch 4 — 44 errors, → 100%) #1408 — batch 4: the remaining tail (44 errors)
Frontend — stores to TypeScript (highest value; most component unknown types originate here)
- [DevEx]: Convert the snackbar store to TypeScript #1422 —
snackbarstore, 38 lines (good first issue) - [DevEx]: Convert the myGames store to TypeScript #1423 —
myGamesstore, 67 lines (good first issue) - [DevEx]: Convert the gameHistory store to TypeScript #1424 —
gameHistorystore, 90 lines (good first issue) - [DevEx]: Convert the gameList store to TypeScript (and to the setup syntax) #1425 —
gameListstore, 124 lines (also needs the setup syntax) - [DevEx]: Convert the auth store to TypeScript (and to the setup syntax) #1426 —
authstore, 171 lines (also needs the setup syntax) - [DevEx]: Convert the game store to TypeScript #1427 —
gamestore, 935 lines (do last; three open PRs touch it)
Close the loop
- [DevEx]: Enable checkJs for the frontend #1409 — Enable
checkJsfor the frontend
Type declarations are written in TypeScript, not JSDoc
One convention, so reviewers have something to point at:
.vuecomponents —<script setup lang="ts">withdefineProps<T>(). Not/** @type {import('vue').PropType<T>} */.- Frontend
.js— convert to.ts. Stores, plugins, router, util. - Backend — stays
.jsundercheckJs. Type information comes from inference plus.d.tsfiles ([DevEx]: Declare the Sails globals for type checking (262 → 27 errors) #1401), not from JSDoc annotations. - JSDoc comments stay as documentation. What is dropped is using them to declare types.
This is a real constraint on the backend, worth naming: checkJs without declared types catches only what it can infer, which is the weaker half — the store experiment above showed it missing 2 of 4 realistic bugs. The backend accepts that for now, and the trade is better there than it looks, because its highest-value type information is inferable: the three shipped bugs checkJs found in get-log.js (#1386, #1398) were caught with zero annotations, purely from resolveJsonModule inferring the JSON enum literal types. Sails' own inputs blocks declare helper arguments at runtime as well.
If that proves too thin once the backend is at zero and the frontend is done, converting the backend to TS is a future phase — not currently planned.
CI arrives early, not last
checkJs governs only .js files — .ts and <script setup lang="ts"> are checked regardless. Measured on main with checkJs: false: 0 errors on both halves, while deliberate errors in a .ts file, an api/**/*.ts file, and a lang="ts" SFC were all still caught.
So both CI jobs land green in #1400, at the start. Every file converted by the store issues (#1422–#1427) and the component issues (#1428, #1405–#1408, #1412) is type-checked from the moment it merges, rather than waiting for #1409 at the end of the plan. #1404 and #1409 then just flip the flag to extend coverage to the remaining .js.
Why TS on the frontend, checkJs on the backend
checkJs catches contradictions against what it can infer, and goes silent wherever inference bottoms out at any. Four realistic bugs injected into src/stores/game.js:
| Bug | checkJs |
+noImplicitAny |
TS |
|---|---|---|---|
newGame.spectatorUsers — typo'd payload field |
❌ | ❌ | ✅ TS2551 ... Did you mean 'spectatingUsers'? |
passes.value = 'three' |
✅ | ✅ | ✅ |
turn.value.toUpperCase() |
✅ | ✅ | ✅ |
first.displayName on a username string |
❌ | ❌ | ✅ TS2339: ... does not exist on type 'string' |
All six stores report 0 errors under checkJs across 1425 lines — silence, not correctness. The two bugs it misses are the ones that ship: a renamed payload field surviving into production is structurally the same bug as #1386. noImplicitAny is not a cheap middle ground — it takes game.js from 0 to 128 TS7006 "annotate this" errors while catching neither.
The backend is different: 243 of its 262 errors are undeclared Sails globals, it is CommonJS, and its helpers follow sails' { friendlyName, inputs, fn } convention. checkJs + a globals.d.ts reaches zero there in two small PRs; converting it would be a large lift for small return.
No build change is required either way. Vite transpiles .ts and <script setup lang="ts"> today via esbuild with no new dependency — verified by building both against the current node_modules, exit 0. But esbuild only strips types: vite build succeeds with const broken: string = 42 in the source, while vue-tsc catches it. Transpiling and checking are separate concerns, which is why #1400 is still needed.
Correction — migration is not the only frontend cause
Measured per batch after #1415 surfaced this: "file is Options API" and "this file's errors are caused by the Options API" are not the same thing.
| Batch | Errors | Migration-fixable | Other |
|---|---|---|---|
#1405 GameView.vue |
137 | 134 | 3 |
| #1406 game dialogs | 119 | 118 | 1 |
| #1407 overlays/rules/stats | 85 | 72 | 13 |
| #1412 six small files | 6 | 1 | 5 |
The large batches are fine — their premise holds at 85–99%. #1412 was wrong and has been corrected in place; its files were chosen by error count and size without checking each error's cause.
Separately, 49 errors across 23 files are untyped props (type: Array / type: Object with no element or shape type, producing unknown at the use site). These appear in <script setup> components too, so they survive every migration batch and need their own pass before #1409 can reach zero.
Notes
- Steps 1–4 deliver the whole value proposition for backend code and are small. If this stalls after step 4, it has still paid for itself.
- Steps 6–9 are ordered by payoff, so stopping early still banks most of the benefit.
eslint.config.js:22carries animport/no-unresolved: offrule for a plugin (eslint-plugin-import) that was never installed. Unrelated, but worth deleting whenever someone is next in that file.
Activity