feat(types): enable TypeScript strict mode and wire tsc into lint gates - #42
Open
erseco wants to merge 30 commits into
Open
feat(types): enable TypeScript strict mode and wire tsc into lint gates#42erseco wants to merge 30 commits into
erseco wants to merge 30 commits into
Conversation
- Enable strict, noImplicitAny, strictBindCallApply, forceConsistentCasingInFileNames and noFallthroughCasesInSwitch in tsconfig.json; switch module resolution to Preserve with bun types. - Add 'make typecheck' (tsc --noEmit) and run it from make lint/fix so type errors fail the same CI gates as Biome. - Fix every type error surfaced by strict mode across src/, scripts/ and iDevice sources (~117 files). - Fix MySQL insertIgnore to use bound parameters instead of sql.lit(). - Parse JWT sub per RFC 7519 (string) with a single userIdFromJwt() helper keeping backward compatibility with numeric tokens. - Replace hand-rolled jsdom typings with @types/jsdom; add @types/bun.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Bundle ReportChanges will decrease total bundle size by 37 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: exporters-bundleAssets Changed:
|
…ynamicDb and expand tests - Enforce positive safe integers in userIdFromJwt() rejecting non-integers, floats, negative values, and exponent strings - Clarify documentation on parsedBody / assertRequestBody as type assertion helper - Isolate and document dynamicDb in DB query helpers while keeping public signatures strongly typed - Add tsconfig.test.json for test suite typechecking - Add tests for userIdFromJwt edge cases, MySQL insertIgnore parameter bindings, and ComponentExporter downloadBlob
Add tests and small extractable helpers for the 23 uncovered patch lines (cookie auth, Blob/Uint8Array/ArrayBuffer payloads, Mermaid SVG polyfills, admin ELPX download, and WebSocket metadata guards).
Extract socketQueryToken/socketDocName so both branches of the Elysia query/params fallbacks are unit-tested, which was the last Codecov patch miss in yjs-websocket.ts.
- Add src/auth/types.ts as the canonical home of JwtPayload and the new AuthenticatedIdentity; sub is parsed to a numeric userId exactly once (userIdFromJwt / toAuthenticatedIdentity). - verifyToken() now returns an AuthenticatedIdentity; VerifiedJwtPayload is gone. - withJwtAuth() derives "identity" instead of "jwtPayload"; the inline derives in yjs, yjs-debug, websocket-info and admin collapse into it. - Guards (requireAuth/requireAdmin/requireAnyRole/isSelfModification) work on identities; all payload re-parses at call sites are removed. - Drop the duplicated JwtPayload from request-payloads.ts. - Fix pre-existing assets API v1 spec isolation (002 migration is not idempotent against an already-migrated DB).
ExportAsset.data, ExportResult.data, ZipProvider.addFile/generateAsync and ZipArchive no longer accept or return Blob. The unified provider (FflateZipProvider) always stored Uint8Array and could not even accept a Blob synchronously, so the wider interface only forced instanceof checks, casts and dual-branch conversions across the pipeline. Blobs are now created exclusively at the browser boundary via the shared blobFromBytes() helper (downloads, object URLs, IndexedDB storage); BrowserAssetProvider/BrowserResourceProvider keep converting to Uint8Array at their own frontier as before.
socketQueryToken/socketDocName existed only to chase branch coverage; the two one-liners are back inline where they belong. The valuable behavioral test (non-string token closes the socket) stays.
IdeviceFileUploadRequest now describes what clients actually send: base64 payloads in "file" or "base64String", createThumbnail as boolean or 'true' string, optional odeSessionId, and raw-string bodies for the large-upload endpoint. This removes the as-unknown-as casts in both handlers. Also replaces String(dataParts[1]) -- which could silently produce the literal string "undefined" for malformed input -- with an explicit 400 on an empty base64 segment, and drops leftover debug logging from the upload path.
tsconfig.test.json was never executed ("typecheck" runs plain
tsc --noEmit) and typechecking all specs is not viable today (~5k
pre-existing strict violations in test mocks), so the dead file goes.
The main config keeps only options that change behavior under strict:
noImplicitAny/strictBindCallApply are implied by "strict", and
allowSyntheticDefaultImports/esModuleInterop by module Preserve.
Emission options (declaration, removeComments, sourceMap, outDir,
incremental) date from when tsc compiled the project; the real build is
bun build and tsc only runs with --noEmit.
…lias One name for the body type assertion, chosen because it makes explicit that no runtime validation happens. The trivial test asserting that a cast returns the same object is removed with it.
The YjsSocket unification left two relics behind: the deprecated WsData alias (kept only so a spec would not have to change) and an exported Room interface in types.ts that nothing consumes -- room-manager keeps its own richer Room. Tests now use YjsSocketData, the production type.
…ndReturn Same pattern as updateWhereAndReturn: the untyped boundary lives in one private helper and the public typed wrappers delegate, removing the 'as never' casts.
Keep this PR's unused IdeviceConfig import drop and take exelearning#2037's zstdCompressBuffer tests in place of gzipBuffer. Strict tsc stays clean.
Keep YjsSocket typing from this PR and take exelearning#2315's clientId-keyed connection map, readyState-during-await guard, and verifyPassword.
Keep MATHJAX_BASE_PATH from exelearning#2351 so static mode does not rebuild the MathJax config. The a11y/strict markup from this branch (button types, section, optional chaining) already merged.
Keep both `typecheck` (this branch) and `update-licenses-check` (v4.0.4) on the `make lint` gate.
`src/shared/block-icon.ts` uses Object.hasOwn, which is ES2022. Raise the tsconfig target so `make typecheck` stays green after the main merge.
make lint failed on update-licenses-check because @types/bun and @types/jsdom were added without regenerating public/libs/README.md. Drop the leftover biome-ignore on blobFromBytes, which is already typed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
TypeScript errors were previously invisible in the toolchain:
tscwas not part of CI gates, and compiler options were mostly relaxed. This PR activates TypeScript strict mode across the application, wirestsc --noEmitintomake lint/make fix, and hardens boundary typing across database queries, authentication helpers, and route payloads.What's included
"strict": true,"noImplicitAny": true,"strictBindCallApply": true,"forceConsistentCasingInFileNames": true,"noFallthroughCasesInSwitch": truewith module resolution"Preserve", official@types/bunand@types/jsdom.make typecheck(tsc --noEmit) wired intomake lint/make fixso type errors break CI like Biome.subparsing: CentralizeduserIdFromJwt()helper insrc/utils/guards.tsthat adheres to RFC 7519 (stringsub), retains backward compatibility for legacy numeric tokens, and strictly enforces positive safe integers (rejecting floats, negatives, exponents, hex, and empty strings).insertIgnore()insrc/db/helpers.tsto use bound parameterized values instead ofsql.lit().parsedBody<T>()/assertRequestBody<T>()as compile-time assertions for collapsed Elysia handler contexts.dynamicDbquery compilation in cross-database helpers while keeping all public helper signatures (insertAndReturn,updateByIdAndReturn,deleteByColumnAndReturn, etc.) 100% strongly typed.YjsSocketandYjsSocketDatainsrc/websocket/types.ts.tsconfig.test.jsonconfiguration for test file typing.Verification
make typecheck(tsc --noEmit): clean (0 errors)make test-unit: 8,367 tests passed (0 failed) across 230 files (100% of 239 files meet the >=90% threshold)make test-integration: 727 tests passed (0 failed)make test-frontend: 100% passed (87.16% coverage)make lint&architecture-check: cleanSimplification pass (post-review)
After review of the strict-mode work, an explicit cleanup pass removed accidental complexity that had crept in to satisfy the compiler and Codecov:
AuthenticatedIdentity(src/auth/types.ts):JwtPayload(wire, stringsub) andAuthenticatedIdentity(internal, numericuserId) are now distinct;subis parsed exactly once viatoAuthenticatedIdentity().verifyToken()returns an identity,withJwtAuth()derivesidentity, the duplicated inline derives in yjs / yjs-debug / websocket-info / admin collapsed into it, guards work on identities, and every re-parse (userIdFromJwt(payload)!) at call sites is gone. The duplicateJwtPayloadinrequest-payloads.tswas deleted.Uint8Arrayas the universal internal binary type in exports:ExportResult.data,ExportAsset.data,ZipProvider/ZipArchiveno longer traffic inBlob; Blobs are created only at the browser frontier via the sharedblobFromBytes()helper.socketQueryToken()/socketDocName()are back inline in the WSopen()handler (the behavioral non-string-token test stays).IdeviceFileUploadRequestnow mirrors the real upload protocol, removing theas unknown ascasts; malformed base64 now returns 400 instead of writing the literal string"undefined"; leftover debug logging removed.tsconfig.test.json(never executed by any gate) deleted;tsconfig.jsonreduced to options that add behavior beyondstrict(emission flags from the pre-bun buildera dropped).parsedBodyrenamed toassertRequestBody, alias removed, cast-triviality test removed.WsDataalias and the unconsumed exportedRoommodel deleted; tests use the productionYjsSocketData.deleteByIdAndReturnroutes through a privatedeleteWhereAndReturn(table, column: string, value: unknown)(same pattern as updates), eliminating theas nevercasts.dynamicDb()stays as the single encapsulated erasure point.Verification (after this pass)
make fix: cleanmake test-unit: green, all files meet the >=90% patch thresholdmake test-integration: 727 passedmake test-e2e: 533 passed (18.0m)make test-e2e-static: 423 passed (13.2m)