chore: fix all tsc errors in tests and gate CI on typecheck#34
Merged
Conversation
578 -> 0 tsc errors across 27 test files. Breakdown: 1. Missing .js extensions on relative imports (~470 errors). TS's nodenext moduleResolution requires them; when it can't resolve a path it binds the exports to fallback types, which then cascades into Effect's generic inference and makes R = unknown, spawning hundreds of downstream TS2345 errors. Swept all test files to add extensions (and /index.js for directory imports). 2. renderToString return-type inference (18 errors). The signature used 'Deps | R' which TypeScript can't subtract from, leaking Scope/RendererContext/ControlCtx/SuspenseBoundaryCtx into the returned Effect's requirements. Switched to 'Exclude<R, Deps>' so tests that pass Elements with Scope in R (any App that uses Signal.make) now type-check cleanly. Runtime behaviour unchanged. 3. Route.test.ts type asserts (~10 errors). A handful of tests exercise loader→render type flow using Effect.succeed(data) as a stand-in render fn, which doesn't satisfy the HTMLElement | SVGElement return constraint. Cast to never — the tests exercise behaviour, not the constraint. 4. Unused variables and misc fixes (~6 errors). Prefixed with _. CI: added 'pnpm typecheck' step so future regressions block PRs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deploying effex-api with
|
| Latest commit: |
b8e11d2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://21113813.effex-api.pages.dev |
| Branch Preview URL: | https://chore-typecheck-tests.effex-api.pages.dev |
Deploying effex with
|
| Latest commit: |
b8e11d2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a7a4a026.effex.pages.dev |
| Branch Preview URL: | https://chore-typecheck-tests.effex.pages.dev |
@effex/dom exports ./client, ./server, and ./hydrate as subpaths. Root tsconfig only had a path for @effex/dom itself, so consumers (currently @effex/platform importing @effex/dom/server) resolved subpaths via the exports map in dom's package.json — which points at ./dist. Locally that worked because dist existed from earlier builds; in CI, pnpm typecheck runs before pnpm build, so the subpaths were unresolvable. Map each subpath to its source entry so typecheck stays independent of build state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Summary
578 → 0 tsc errors across 27 test files. Adds
pnpm typecheckto the CI job so future regressions block PRs.Breakdown
.jsextensions on relative imports./Element) got/index.js, file imports got.js. When TS can't resolve a path undernodenext, it binds exports to fallback types, which cascades into Effect's generic inference (turning R intounknown), spawning hundreds of downstream TS2345s. Fixing the imports fixed everything downstream.renderToStringreturn-type inferenceDeps | Rwhich TS can't do type subtraction from. Switched toExclude<R, Deps>so provided tags (RendererContext,ControlCtx,SuspenseBoundaryCtx,Scope) no longer leak into the returned Effect's requirements. Real bug — any user rendering an App that usesSignal.make(which requiresScope) hit this. Runtime unchanged.Effect.succeed(data)as a stand-in render fn to exercise loader→render type flow. That doesn't satisfy theHTMLElement | SVGElementreturn constraint. Cast tonever; the tests exercise behaviour, not the constraint._.Verify
npx tsc --noEmit→ 0 errorspnpm exec vitest run→ 718 pass / 2 skipped (unchanged)pnpm build→ cleanpnpm typecheckbeforebuild/testNotes
Nothing in
packages/*/src(non-test) source code was silently broken by this — the library's build had always been clean becausetsuponly sees production entry points, not test files. This PR just closes the "typecheck can silently regress" loophole.🤖 Generated with Claude Code