Skip to content

ci: gate packages/types on tsc (reopens #828) - #836

Merged
lilyshen0722 merged 1 commit into
mainfrom
ci/typecheck-packages-types
Aug 4, 2026
Merged

ci: gate packages/types on tsc (reopens #828)#836
lilyshen0722 merged 1 commit into
mainfrom
ci/typecheck-packages-types

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Reopens #828, which was closed unmerged rather than rejected. It was stacked on fix/frontend-typecheck-gate (#826); merging #826 with --delete-branch deleted the base branch, and GitHub auto-closed this PR as a side effect. GitHub refuses to reopen a PR whose base is gone, so this is a fresh PR with the same commit rebased onto main.

packages/types defines a tsc:check script that nothing invokes. It is green today, verified locally against the compiler the backend step already installs:

$ cd packages/types && ../../backend/node_modules/.bin/tsc --noEmit -p tsconfig.json
$ echo $?
0

Gating it now is free — it costs no install, because packages/types has zero dependencies and zero external imports. The moment to add a gate is while the thing is already passing; after it drifts, the same change becomes a migration.

packages/commonly-apps is deliberately not gated here. It defines the same uninvoked tsc:check, but it needs @types/node and is not npm ci'd by this job, so gating it means adding an install step. Its real state is unknown rather than assumed green — that is a separate change with a separate risk.

Follows the same reasoning as #823 (the typecheck gate that skipped middleware/ and six other directories): a check that exists but never runs is not a check.

packages/types defines `typecheck: tsc --noEmit` and nothing has ever
invoked it — the same defined-but-uninvoked shape #826 fixes for the
frontend. It is green today, which is the only moment a gate is free.

Costs no install: the package has zero dependencies and zero external
imports, so it typechecks with the compiler the backend step already
installed. Two lines and a comment.

packages/commonly-apps is deliberately left out. It has the identical
uninvoked `tsc:check` script, but it needs @types/node and this job never
npm ci's it, so gating it means adding an install step. Its real state is
unknown rather than assumed green — reporting it as green off a TS2688
"cannot find type definition file for 'node'" would be reporting an
environment gap as a result.

Mutation-checked: a deliberate `const x: number = "str"` in src/user.ts
fails the step with TS2322 (exit 2), and the step returns to exit 0 once
reverted, so the gate can actually go red.

Stacked on #826 (25e6de7) because it is the only other open PR touching
tests.yml; merging it first avoids a conflict in the same hunk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722
lilyshen0722 merged commit 9347d7b into main Aug 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant