Upgrade TypeScript 2.8 → 6.0.3, and get type checking working at all by toddmedema · Pull Request #932 · ExpeditionRPG/expedition

toddmedema · 2026-09-07T10:03:32Z

Stacked on #931 — review that one first; this branch's diff is against it, not master.

Why

After #931 the repo had a working test suite and a green build, but nothing type-checked. Dropping ts-jest removed the last type check, and tslint is not one. TypeScript had been pinned at 2.8.3 since 2018.

Not 7.0.2

npm view typescript version reports 7.0.2 as latest, but the 7.x npm package is the Go port and exposes no compiler API:

exports["."] → ./lib/version.cjs
$ node -e "const ts=require('typescript'); console.log(Object.keys(ts))"
version,versionMajorMinor          # createProgram / transpileModule: undefined

Tested rather than inferred from peer ranges:

ts-loader@9.6.2 → TypeError: Cannot read properties of undefined (reading 'fileExists')
typescript-eslint@8.69.0 → throws at require() time; no published version, canary included, supports TS 7

So 6.0.3 — the newest TypeScript that can actually build and lint this project.

The result

tsc --noEmit exits 0, from 425 real project errors. Four root causes covered ~300:

TS 6 defaults esModuleInterop to true, which made every import * as x of an export = function non-callable (express(), session(), classNames()). Set explicitly to false — the repo's long-standing intent, and what jest's swc noInterop: true already mirrors.
strictPropertyInitialization now follows strictNullChecks.
Sequelize this-inference: every findOne/create was resolving to Model<unknown, unknown>.
this in object-literal methods is now the literal, not the rebound context.

Toolchain: awesome-typescript-loader (abandoned 2018) → ts-loader 9; tslint (dead 2019) → eslint 10 + typescript-eslint; path aliases moved to webpack resolve.alias; new yarn typecheck and a CI Typecheck step.

No `any` shortcuts

	before	after
`any` annotations	707	701
`as any`	128	107
`@ts-ignore` / `@ts-expect-error` / `as unknown as`	0	0
definite-assignment `!:`	0	127

any went down. The escape hatch actually used is !:, on fields written by a constructor-called initializer (SchemaBase's @field reflection, setupModels()). Three were wrong and are now typed honestly — notably TextAreaDialog.action, which only one subclass assigns, which is exactly why render() reads this.action || 'Submit'.

no-non-null-assertion is on, not off: it reports expression assertions (foo!.bar), not definite-assignment declarations, so the 127 were never a reason to disable it. Shipped code has zero of the dangerous kind.

The regression this nearly shipped

Dropping babel from the loader chain silently moved the browser bundles to target: es6. awesome-typescript-loader had been running babel-preset-env over its output, targeting chrome >= 30 / last 3 iOS versions. services/app is packaged with Cordova — cordova-android@7.1 supports Android 4.4 (WebView = Chrome 33) and cordova-ios@4.5 supports iOS 9, neither of which can parse ES6 class syntax. That is a white screen, not graceful degradation.

tsconfig.browser.json restores the ES5 emit for the four browser bundles; tsc --noEmit stays on es6, and services/api keeps the root config since it runs on node. Verified in the artifact: app/www/bundle.js contains our classes in ES5 form with no class syntax.

Bugs the type checker found

lib/oauth2.ts res.end(401, 'Unauthorized') — Express reads that as (chunk, encoding), so the status stayed 200. Spellcheck.tsx '[^\s]*' written in a string, collapsing to [^s]* ("anything but a lowercase s"). Client.tsx reading .client/.instance off a JSON.parse SyntaxError. SavedQuests.tsx a .catch() placed before the parsing .then(), so after a network failure the XML parse ran on a dispatched action. Plus unchecked findOne() results, a parseInt() over a numeric column, and theme: CardThemeType | {} where every string satisfies {}.

Separately, saveQuestForOffline's quota check used a bare indexOf(...), truthy for -1, so every save failure reported "out of storage space" and the generic error was unreachable — and since the substring is always preceded by "Error: ", index 0 never occurred either. Fixed, with both branches now tested; mutation-checked by restoring the bug.

Verification

tsc --noEmit    exit 0        (TypeScript 6.0.3)
yarn lint       exit 0        (eslint 10 + typescript-eslint, 615 .ts/.tsx files)
jest            1298 passed, 0 failed, 181 suites
builds          5/5 services green

Review

Two review passes, both of which also fixed what they found. Everything below was verified by
execution, not by reading:

All eight type-checker-found bug fixes confirmed correct against their callers. The
SavedQuests promise-chain fix is mutation-checked: restoring the old .catch()-before-.then()
shape fails the new test with Expected: 1, Received: 2 — it had been emitting a second, wrong
"out of storage space" snackbar on network failure.
The three alias copies agree. All 69 alias-prefixed specifiers were resolved under each
mapping's own rules: 0 mismatches. Rather than leave three hand-written copies, jest.config.js
now derives its moduleNameMapper from shared/webpack.aliases.js, and a new guard test asserts
tsconfig.json's paths (which must stay hand-written — tsc reads it directly) names the same roots.
@types pins are legitimate dedup, not masked incompatibilities.
tsconfig types had a real gap: the global CheerioAPI interface used by a dozen
require('cheerio') as CheerioAPI sites was only reaching the program by accident, via
@types/enzyme. Now listed explicitly.

Worth knowing before merge

The test suite is not type-checked at all. tsconfig.json excludes **/*.test.ts(x); running
tsc over tsconfig.eslint.json (which includes them) yields 823 errors. This is pre-existing
and not introduced here — but it means "type checking works now" is true only of shipped code.
Fixing it is a separate, sizeable task.
@types/passport is pinned at 1.0.7 to match the installed passport@0.4.0, and 0.4.0
predates the CVE-2022-25896 session-fixation fix (landed in passport 0.6.0). The pin is correct
for the code as it stands; the underlying upgrade is out of scope here and wants its own PR.
Columns<T> types every column as always present, which is wrong under attributes: [...]
projections. Nothing does that today.
oauth2.ts's 401 path has no test (the file has only pre-existing test.skip stubs), and
Events.ts's Number(...) coercion is only exercised against sqlite, which returns numbers —
the string-shaped Postgres case it exists for is untested in CI.
transpileOnly: true means the build does not type-check; yarn typecheck is authoritative and
runs in CI.
Much of the diff is prettier reformatting from the pre-commit hook. ?w=1 helps.
🤖 Generated with Claude Code
_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

typescript 2.8.3 (pinned since 2018) -> 6.0.3, and with it the two dead tools that were holding it there. TypeScript 7.0.2 is `latest`, but it is the Go port and its npm package exposes no compiler API at all -- the package's main export resolves to lib/version.cjs, whose only keys are `version` and `versionMajorMinor`. Everything that embeds tsc breaks on it, verified rather than assumed: ts-loader 9.6.2: TypeError: Cannot read properties of undefined (reading 'fileExists') at findConfigFile typescript-eslint 8.69.0: TypeError: Cannot read properties of undefined (reading 'Cjs') -- thrown at require() time Both work on 6.0.3, the newest release that keeps a type-aware linter, so that is where this lands. typescript-eslint declares `typescript: >=4.8.4 <6.1.0` and no published version, canary included, supports 7. awesome-typescript-loader (last released 2018) -> ts-loader 9. It has no `useBabel`, so the `shared/*`, `app/*` and `api/*` aliases that the babel module-resolver plugin used to rewrite now come from webpack `resolve.alias` in shared/webpack.aliases.js, shared by the browser and API configs. That mapping now lives in three places that have to agree and the file says which. Dropping babel from the build also makes babel-core, babel-preset-env and the two plugins dead weight. tslint 5.8 (deprecated 2019, last release ever 6.1.3) -> eslint 10 + typescript-eslint, with tslint.json's rules ported one for one in eslint.config.js. The linter is now genuinely type-aware: tslint built a Program but the webpack loader ran it with `typeCheck: false`, so no type-aware rule ever fired. Test files lint against tsconfig.eslint.json because tsconfig.json excludes them from `tsc`. Getting `yarn lint` back to zero meant real fixes, not suppressions: 29 case clauses that leaked let/const across cases now have blocks, six `let body` that are never reassigned are const, five hasOwnProperty calls go through Object.prototype, and three genuine variable shadows are renamed. Two bugs the linter found are noted in their own commits. tsconfig changes are all TS 6 migrations, none of them relax a check: `baseUrl` is gone (6.0 resolves `paths` relative to the config file), `rootDir` is pinned because ts-loader compiles file-at-a-time and 6.0 infers a different common source directory for each one, and `types` is explicit because 6.0 stopped pulling all of node_modules/@types into global scope. noImplicitAny, strictNullChecks and noUnusedLocals are untouched. CI gains a Typecheck step. `tsc --noEmit` is now the authoritative type check, so ts-loader runs with transpileOnly in all five builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three separate causes accounted for 102 of the 258 project errors. esModuleInterop now defaults to `true` in TypeScript 6. That silently turns every `import * as x` of an `export =` function into a non-callable namespace object, so `express()`, `session()`, `memoize()`, `classNames()` and `createRavenMiddleware()` all stopped type-checking, and the emit would have started inserting interop helpers that jest's swc transform (`noInterop: true`) and the existing bundles do not expect. Setting it explicitly to `false` is what the repo has always meant; it is not a relaxation. Sequelize's statics are declared `this: {new(): M} & typeof Model`. Every model alias in Database.ts was written with the object literal first, and TS 6 now resolves `M` against `typeof Model`'s own construct signature instead of ours -- `findOne`/`findAll`/`create` all came back as `Model<unknown, unknown>` and lost `dataValues`. Writing the intersection as `typeof Sequelize.Model & {new(): I}` restores the intended inference. Nothing about the runtime changes. The combat scope object literal is rebound to the quest context by evaluateOp (`ctx.scope._[k].bind(ctx)`), so `this` is a TemplateContext. TS 6 infers `this` in an object-literal method as the literal itself, which has no `scope`, `templates` or `seed`. Each method now declares `this: TemplateContext`, which is what was always true. Also: the seven imports `noUnusedLocals` flagged are gone, and quests/src/React.tsx no longer declares `require` twice (harmless as `var`, an error once the earlier lint fix made it `let`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Down from 156 project errors to 88. `instance.get('partition')` was coming back as `unknown`: Sequelize's string overload of `get()` returns exactly that, and the precise overload keys off `keyof this`, which for these interfaces was only the Model surface. Mixing `Columns<T>` -- the schema class minus its SchemaBase bookkeeping and `withoutDefaults()` -- into each *Instance interface puts the columns on `this`, so the typed overload applies. That is what the interfaces always claimed to describe. remoteify declared `dispatch?` and `getState?: () => AppState`, but the multiplayer middleware calls `fn(args, dispatch, getState)` with both always present and with the real store's getState, which returns AppStateWithHistory. 24 actions that name the parameters the way they are actually called were unassignable. Making the signature match the one call site fixes all of them at once. In api/src/Handlers.ts: `req.query.x` is `string | string[] | ParsedQs | ParsedQs[] | undefined` under @types/express 4.17, so publish() now goes through queryString / queryNumber / queryBoolean helpers. The coercions match what Joi was already doing to these values ('true'/'false' -> boolean, numeric strings -> number), and a repeated or nested parameter now reads as absent instead of being handed to the schema. The quest search filter gained a `q is Quest` predicate so the following .map() is honest, and the feedback switch narrows `req.params.type` through its own cases instead of asserting `as FeedbackType` up front -- one fewer assertion than before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Down to 36 project errors. Nothing here is a suppression; each one was an annotation that disagreed with the code underneath it. Sequelize's findOne() returns `Instance | null` and every one of these callbacks already branched on the missing row -- only the parameter annotations claimed non-null. getSessionBySecret's declared return type said `Bluebird<SessionInstance>` while its body returned `result || null`. ws.send()'s callback receives `err?: Error`; seven handlers declared `(e: Error)`. Bluebird's .filter() cannot narrow an element type, so the multiplayer session listing now filters inside .then() with a predicate that can. Real bugs the checker turned up: * lib/oauth2.ts called `res.end(401, 'Unauthorized')`. Express reads that as (chunk, encoding): it sent the body "401" with a bogus encoding, left the status at 200, and then fell through and dereferenced the missing user. Now `res.status(401).end(...)` with the return it always needed. * shared/multiplayer/Client.tsx read `e.client` and `e.instance` off the SyntaxError from a failed JSON.parse. Both were always undefined, in fields MultiplayerEvent requires. * models/multiplayer/Events.ts ran `parseInt()` over the numeric `id` column. * models/Quests.ts updateQuestRatings dereferenced a findOne() result without checking it. Also: @types/express was duplicated at 4.16.0 under express-session, passport and sinon-express-mock, so express's own RequestHandler was not assignable to its own app.use(). Pinned through `resolutions`. @types/express-session moved to 1.18, which needs SessionData augmented rather than arbitrary properties -- oauth2.ts now declares the five fields this service actually stores. @types/passport is pinned to 1.0.7, the last release whose `logout()` matches the installed passport 0.4. QDLMode's ace constructor functions get explicit `this` parameters, typed with interfaces covering exactly the members the file touches -- `brace` ships no typings and TS 6 no longer lets `this` default to any. Typing $rules also let the linter catch a for-in over an array there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Redux calls a reducer with `undefined` on the first dispatch; four combined reducers declared the state parameter non-optional even though every one of them opened with `state = state || {}`. `_committed` in AppStateWithHistory is built by stripMultiplayerStateAndSettings, which deletes four fields; it is a Partial and is now typed as one, so the deletes are legal instead of being errors against required properties. ContentSetsType is keyed by the Expansion enum, not by string, so Object.keys() could not index it. Walking `enumValues(Expansion)` instead keeps the key type and gives the same answer, since an absent set reads as undefined either way. Promise executors and Array.reduce() calls that were relying on an inferred accumulator now say what they produce. Real bugs found here: * actions/SavedQuests.tsx had a `.catch()` before the `.then()` that parses the response. A .catch() resolves the chain, so after a network failure the parse ran on the dispatched snackbar action rather than on quest XML. It is a rejection handler on the same .then() now, which is what the code meant. * Roleplay's theme parameter is `CardThemeType | {}`, and every string satisfies `{}`, so `typeof theme === 'string'` narrowed to string and any string reached a three-value union. It checks the values. * models/Quests.ts set `where.published = {[Op.ne]: null}` a second time inside `if (params.owner)`; the initializer four lines up already did exactly that. @types/react was duplicated at 16.4.6 under four packages, which is why a ReactElement from react-transition-group was not a ReactElement to React.cloneElement. Pinned through `resolutions`, with @types/react-transition-group moved to 2.9.2. `tsc --noEmit` now exits 0. noImplicitAny, strictNullChecks and noUnusedLocals are all still on, plus TS 6's strictPropertyInitialization and useUnknownInCatchVariables, which this branch adopted rather than switched off. The count of lines containing `any` went down over the branch (1021 -> 985; `as any` 352 -> 301). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

getLargestEventID dropped `parseInt(e.get('id'), 10)` when `id` became typed as `number`. The type is only true under sqlite (what the tests run on): `Event.id` is a BIGINT column and node-postgres returns int8 as a string, so in production the value stayed a string and Chaos.ts's `latestID + 1` produced string concatenation. `Number()` restores the coercion and typechecks against the declared type. SavedQuests' out-of-storage branch tested `indexOf('exceeded the quota')` without comparing to -1, so it was truthy for every error and reported "out of storage space" for network and parse failures too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The browser bundles regressed to `target: es6`. awesome-typescript-loader used to run babel over its own output, and package.json's babel section targeted `chrome >= 30` / `last 3 iOS versions`, so every shipped bundle was ES5. Dropping babel from the loader chain started shipping ES6 class syntax to those same browsers. services/app is packaged with Cordova: cordova-android@7.1 supports Android 4.4 (WebView = Chrome 33) and cordova-ios@4.5 supports iOS 9, neither of which can parse it -- a white screen, not a graceful degradation. tsconfig.browser.json restores the es5 emit for the four browser bundles; `tsc --noEmit` stays on es6, and services/api keeps the root config since it runs on node. Verified: all five bundles build, and app/www/bundle.js now contains our classes in ES5 form with no `class` syntax. no-non-null-assertion is back on. The rule reports expression assertions (`foo!.bar`), not the definite-assignment declarations (`foo!: T`) this branch added, so the 127 schema fields were never a reason to disable it. Shipped code has zero of the expression kind, so it is free today and stops the dangerous form creeping in. Tests are exempted: they legitimately assert on a value they have just checked. Three assertions were wrong and are now typed honestly: - Dialogs.tsx TextAreaDialog.action: only ExitQuestDialog assigns it, which is why render() reads `this.action || 'Submit'`. Now optional. - QDLParser.reverseLookup: given a real initializer. - Connection.sendStatus: declared but never assigned or called; removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The quota check used a bare `indexOf(...)`, which is truthy for -1, so the out-of-storage branch matched every failure and the generic "Error saving quest" snackbar was unreachable. Worse, the substring is always preceded by "Error: ", so index 0 never occurred either -- real, reportable save errors were silently reported as being out of space, which the comment there explicitly says they should not be. The condition was fixed in the previous commit; these are the tests that hold it. Mutation-checked: restoring the bare indexOf() fails the generic case with `Expected "Error saving quest" / Received "Couldn't save; out of storage space."`, while the quota case still passes -- that branch was only ever correct by accident. saveQuestForOffline offers no seam for injecting storage, so the mocks are scoped with resetModules()/doMock() rather than a file-wide jest.mock(); the rest of this file relies on the real LocalStorage. Note a fetchLocal rejection does not reach this .catch -- it is handled by the rejection handler on the same .then() and produces the network-error snackbar -- so both cases are driven through the storage path. Replaces the `test.skip('storage errors are shown in snackbar')` stub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The .catch()-before-.then() rewrite in this branch changed runtime behaviour and had no test. The old shape resolved the chain, so the parsing .then() ran on the dispatched action and produced a second, wrong snackbar ("out of storage space") plus a store attempt; the rejection handler on the same .then() produces exactly one. Mutation-checked: restoring the old .catch() ordering fails this test with "Expected: 1, Received: 2". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The shared/app/api mapping lived in three hand-written places (tsconfig.json paths, jest.config.js moduleNameMapper, shared/webpack.aliases.js). All three agree today -- verified by resolving every one of the 69 alias-prefixed specifiers in the repo under each mapping and comparing the resolved file -- but nothing stopped them drifting, and a drift shows up only at runtime in one bundle. jest.config.js now derives its mapper from shared/webpack.aliases.js, so two of the three copies are one copy. tsconfig.json's `paths` has to stay hand-written because tsc reads it directly, so a test asserts it still names the same roots. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The explicit `types` array replaced TypeScript's automatic inclusion of everything in node_modules/@types. Nothing in the array is missing for shipped code -- `tsc --noEmit` is clean -- but the global `CheerioAPI` interface that a dozen `require('cheerio') as CheerioAPI` sites annotate with was reaching the program only because services/app/src/Testing.tsx imports enzyme and @types/enzyme imports cheerio. Remove that unrelated import and services/app stops compiling. Naming it makes the dependency real instead of transitive. No other @types package in the repo supplies an ambient global that is used without an import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude added 8 commits September 6, 2026 22:09

toddmedema changed the title ~~claude/typescript upgrade~~ Upgrade TypeScript 2.8 → 6.0.3, and get type checking working at all Sep 7, 2026

claude added 3 commits September 7, 2026 12:21

toddmedema mentioned this pull request Sep 8, 2026

Upgrade every outdated dependency: 919 -> 189 vulnerabilities #934

Open

toddmedema marked this pull request as ready for review September 9, 2026 16:39

Navigation Menu

Upgrade TypeScript 2.8 → 6.0.3, and get type checking working at all - #932

Upgrade TypeScript 2.8 → 6.0.3, and get type checking working at all#932
toddmedema wants to merge 11 commits into
claude/modernize-testing-setup-49a843from
claude/typescript-upgrade

toddmedema commented Sep 7, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

toddmedema commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Not 7.0.2

The result

No any shortcuts

The regression this nearly shipped

Bugs the type checker found

Verification

Review

Worth knowing before merge

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

toddmedema commented Sep 7, 2026 •

edited

Loading

No `any` shortcuts