fix(portal): make type-check actually type-check - #130
Merged
Conversation
apps/portal/tsconfig.json is a solution-style config with "files": [] and project references. Invoked without -b, tsc honours files: [], checks zero files, and exits 0. So both "type-check": "tsc --noEmit" and the tsc in "build": "tsc && vite build" guarded nothing, and a green CI proved nothing. Vite compiles with esbuild, which does no type checking, and ESLint does not enable no-undef for TS files, so an undefined global could pass lint, type-check and build. Verified: an undefined call in a component passes both yarn lint and vite build with exit 0. Switch both scripts to build mode (tsc -b), which walks the referenced projects. Both referenced configs already set noEmit: true, so nothing is emitted but .tsbuildinfo under apps/portal/node_modules/.tmp/ (gitignored), which keeps incremental caching that --noEmit would discard. Bump the portal's typescript pin 5.7.3 -> 5.8.3 to match every other workspace. tsconfig.node.json already sets erasableSyntaxOnly, a TS 5.8 option, so build mode failed on 5.7.3 with TS5023. That option had never been read by the portal's own compiler, for the same reason. Fix the two pre-existing errors the working check now exposes: - TransactionForm.tsx: ReactNode is type-only under verbatimModuleSyntax (TS1484) - theme-store.ts: unused @ts-expect-error (TS2578). MediaQueryList.addListener is present in lib.dom.d.ts, deprecated rather than missing, so the directive suppressed nothing. The Safari runtime fallback and try/catch are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
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.
The problem
apps/portal/tsconfig.jsonis a solution-style config: it has"files": []and delegates to project references.{ "files": [], "references": [{ "path": "./tsconfig.app.json" }, { "path": "./tsconfig.node.json" }] }Invoked without
-b,tschonoursfiles: []literally: it checks zero files and exits 0. So both of these guarded nothing at all:tsc --noEmitprinted no output and exited 0 because there was nothing in its input set. Build mode (-b) is what walks thereferencesarray; plain mode ignores it.Nothing else covered the gap either. Vite compiles with esbuild, which strips types without checking them, and our ESLint config does not enable
no-undeffor TS files. So an undefined global could pass lint, type-check, and build. Demonstrated on this branch by putting an undefined call into a component:yarn lintno-undefoff for TS)vite buildtype-check(before)type-check(after)TS2304: Cannot find name ...CI has been running the vacuous command all along, showing a green "Type check" step that verified nothing.
The fix
Both scripts move to build mode:
Plain
tsc -brather thantsc -b --noEmit: both referenced configs already setnoEmit: true, so the only thing written is.tsbuildinfointoapps/portal/node_modules/.tmp/(gitignored). Keeping it preserves incremental caching that--noEmitwould discard.TypeScript pin 5.7.3 -> 5.8.3
The portal was the only workspace pinning an exact
typescriptversion; every other one floats to 5.8.3. Its owntsconfig.node.jsonalready setserasableSyntaxOnly, a TS 5.8 option, so under build mode the workspace-local 5.7.3 failed withTS5023: Unknown compiler option 'erasableSyntaxOnly'. Build mode reads both referenced configs, where plain mode never touchedtsconfig.node.json.That option has therefore never once been read by the portal's own compiler, which is the same root cause wearing a different hat.
Two pre-existing errors, newly exposed
These are not introduced by this PR. They already existed on
mainand were invisible because the check was vacuous. The PR cannot be green without them:components/transaction/TransactionForm.tsx:1—TS1484.ReactNodeis type-only andverbatimModuleSyntaxis on:import React, { type ReactNode } from 'react';stores/theme-store.ts:87—TS2578, unused@ts-expect-error.MediaQueryList.addListeneris declared inlib.dom.d.ts(marked@deprecated, not missing), so the directive suppressed nothing and became an error itself. Only the directive line is deleted; the Safari runtime fallback and surroundingtry/catchare untouched, and since the directive is compile-time only, the built output is unchanged.Heads-up: open PRs with type errors will now fail CI
This is the intended effect, not a regression. Any open PR carrying type errors was passing CI on a check that inspected nothing; those errors now surface where they should.
Confirmed against #129's head (
6fb99ae), which has three genuine errors. With the fixed script they all report:Left untouched here; they belong to that PR. Expect it to go red after this merges and to need those three fixed (plus a rebase to pick up the two above).
Note on CI scope
Worth recording, since it came up: the portal is now gated at two CI steps, since the
Buildstep runs every workspace'sbuildand the portal's now begins withtsc -b.I considered switching the
Type checkstep fromyarn workspace @auto-portal/portal type-checkto rootyarn type-check, and recommend against it.shared-lib,shared-state, andshared-uiall use"build": "tsc -b", so CI's Build step already type-checks them (verified: injecting a type error intoshared-libfails its build withTS2322, exit 1). Widening the step would add little real coverage while fanning out toservices/staking-indexer, whosesrc/typesis generated. No workflow change in this PR.Separately, and out of scope: CI never runs
yarn test. The steps are Lint, Prettier format check, Type check, Build. The portal's 35 tests pass but nothing enforces that; resolving it first needs the@polkadot/typesconflict in the indexer/worker workspaces sorted out. Probably worth its own issue.Verification
yarn workspace @auto-portal/portal type-check— exit 0, with both.tsbuildinfofiles written from a cleaned.tmp/, so it is doing real work rather than exiting earlyyarn lint— exit 0yarn build— exit 0yarn workspace @auto-portal/portal test:run— 35/35 passyarn install --immutable— exit 0, lockfile is CI-cleantype-checkfails withTS2304while lint andvite buildboth pass, then revertedTwo root suite commands fail identically on unmodified
main(confirmed by stashing this work), so they are pre-existing and unrelated:yarn format:check— 15 gitignored SubQuery codegen files underservices/staking-indexer/src/types/. None of the files in this diff are flagged.yarn test—@polkadot/typesversion conflict in the indexer/worker workspaces. CI does not run this.🤖 Generated with Claude Code