chore: upgrade tsgo and gate dev/build flows#962
Conversation
📝 WalkthroughWalkthrough此 PR 在本地与 CI 流程中新增 TypeScript 类型检查步骤，固定/更新若干依赖与脚本，扩展 TypeScript 声明与包含范围，调整 CI 步骤条件，并更新 Docker 忽略项与测试中的 logger mock 与文档说明。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's type-checking infrastructure by upgrading to the latest Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully upgrades tsgo to a newer version and integrates it into the development and build workflows as a pre-flight type-checking step. The changes are well-documented, including performance metrics and fixes for issues uncovered during the migration. The pinning of dependencies to isolate the upgrade is a good practice. I have one suggestion regarding the developer experience of the dev script to provide continuous type-checking.
| "scripts": { | ||
| "dev": "next dev --port 13500", | ||
| "build": "next build && (node scripts/copy-version-to-standalone.cjs || bun scripts/copy-version-to-standalone.cjs)", | ||
| "dev": "tsgo -p tsconfig.json --noEmit && next dev --port 13500", |
There was a problem hiding this comment.
Gating the dev script with tsgo is a good step for ensuring type safety. However, as you've noted, it increases startup time, and the check only runs once. For a better developer experience with continuous type-checking, you could offer a watch mode. While the current implementation acts as a pre-flight check, developers would benefit from immediate feedback from tsgo as they save files. A common pattern is to have a separate typecheck:watch script using tsgo -p tsconfig.json --noEmit --watch that developers can run in another terminal.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - name: Install dependencies, type check, and format code | ||
| if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch' | ||
| run: | | ||
| bun install | ||
| bun run typecheck | ||
| bun run format |
There was a problem hiding this comment.
The bun run typecheck call is placed inside a step that has an explicit if: condition. In GitHub Actions, an explicit if: expression replaces the implicit success() guard. As a result, all downstream steps — including "Commit VERSION and formatted code", "Create and push tag", and "Build and push Docker image" — also carry if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch' without a success() check. If bun run typecheck exits non-zero:
- This step is marked failed.
- The next step ("Commit VERSION and formatted code") evaluates its own
if:expression independently of the failure. Sincesteps.check.outputs.needs_bumpwas already set to'true', the condition is still true, so it runs anyway. - The same applies to "Create and push tag" and "Build and push Docker image" — a release can be cut even on a type-error build.
The same scenario in dev.yml is safe because those steps carry no explicit if: and therefore default to if: success().
Suggested fix — add success() to every downstream if: that should be blocked by a failed typecheck:
- name: Commit VERSION and formatted code
if: (steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch') && success()Or, more robustly, move the typecheck into its own earlier step (before "Update VERSION file") so a failure causes the whole job to terminate under the default success() policy:
- name: Install dependencies
if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'
run: bun install
- name: Type check
if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'
run: bun run typecheck
- name: Format code
if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'
run: bun run formatSplitting the step causes GitHub Actions to re-apply the default success() gate between each step, so a typecheck failure prevents any later step from running regardless of its own if: expression.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 201-206
Comment:
**Typecheck failure does not gate the release pipeline**
The `bun run typecheck` call is placed inside a step that has an explicit `if:` condition. In GitHub Actions, an explicit `if:` expression **replaces** the implicit `success()` guard. As a result, all downstream steps — including "Commit VERSION and formatted code", "Create and push tag", and "Build and push Docker image" — also carry `if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'` without a `success()` check. If `bun run typecheck` exits non-zero:
1. This step is marked failed.
2. The next step ("Commit VERSION and formatted code") evaluates its own `if:` expression independently of the failure. Since `steps.check.outputs.needs_bump` was already set to `'true'`, the condition is still true, so it **runs anyway**.
3. The same applies to "Create and push tag" and "Build and push Docker image" — a release can be cut even on a type-error build.
The same scenario in `dev.yml` is safe because those steps carry no explicit `if:` and therefore default to `if: success()`.
Suggested fix — add `success()` to every downstream `if:` that should be blocked by a failed typecheck:
```yaml
- name: Commit VERSION and formatted code
if: (steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch') && success()
```
Or, more robustly, move the typecheck into its own earlier step (before "Update VERSION file") so a failure causes the whole job to terminate under the default `success()` policy:
```yaml
- name: Install dependencies
if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'
run: bun install
- name: Type check
if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'
run: bun run typecheck
- name: Format code
if: steps.check.outputs.needs_bump == 'true' || github.event_name == 'workflow_dispatch'
run: bun run format
```
Splitting the step causes GitHub Actions to re-apply the default `success()` gate between each step, so a `typecheck` failure prevents any later step from running regardless of its own `if:` expression.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/app/v1/url.test.ts (1)
3-10: Mock 扩展正确，可选择性地考虑完善。添加
info和warn方法修复了测试问题。不过当前 mock 仍缺少trace和fatal方法（Pino logger 的完整接口）。如果将来代码调用这些方法，可能会再次导致测试失败。🔧 可选：完善 logger mock
vi.mock("@/lib/logger", () => ({ logger: { + trace: vi.fn(), debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), error: vi.fn(), - info: vi.fn(), - warn: vi.fn(), + fatal: vi.fn(), }, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/app/v1/url.test.ts` around lines 3 - 10, The logger mock in tests/unit/app/v1/url.test.ts only includes debug/error/info/warn and should be expanded to include the remaining Pino methods to avoid future test breaks; update the vi.mock for "@/lib/logger" (the exported logger object) to add trace and fatal (as vi.fn()) so any calls to logger.trace or logger.fatal during tests are stubbed and won't throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/app/v1/url.test.ts`:
- Around line 3-10: The logger mock in tests/unit/app/v1/url.test.ts only
includes debug/error/info/warn and should be expanded to include the remaining
Pino methods to avoid future test breaks; update the vi.mock for "@/lib/logger"
(the exported logger object) to add trace and fatal (as vi.fn()) so any calls to
logger.trace or logger.fatal during tests are stubbed and won't throw.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee998bf8-b40d-4df4-9fb7-090cc83c1fe2
📒 Files selected for processing (8)
.dockerignore.github/workflows/dev.yml.github/workflows/release.ymlCLAUDE.mdpackage.jsonsrc/types/css.d.tstests/unit/app/v1/url.test.tstsconfig.json
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: S
- Lines changed: 30
- Files changed: 8
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#962and pulled metadata + full diff (8files,30total lines changed) for review. - Applied the PR size label:
size/S. - Submitted a PR review comment summary (
gh pr review) — no diff-line issues met the reporting threshold, so no inline review comments were posted.
github-actions
Bot
added
size/XS
There was a problem hiding this comment.
Code Review Summary
This PR correctly upgrades the TypeScript checker to tsgo and integrates typecheck gates into the dev/build workflows. All changes are focused and well-implemented.
PR Size: XS
- Lines changed: 30 (18 additions, 12 deletions)
- Files changed: 8
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Notes
-
CSS Type Declaration (
src/types/css.d.ts): Correct fix for TS2882 side-effect import regression. The ambient module declarationdeclare module "*.css";is the standard solution. -
Test Mock Fix (
tests/unit/app/v1/url.test.ts): Correctly completes the logger mock withinfoandwarnstubs to resolve the pre-existing TypeError. -
Docker Ignore (
.dockerignore): Appropriate additions to exclude local development volume data from Docker build context. -
Workflow Changes: Both
dev.ymlandrelease.ymlcorrectly addbun run typecheckto the pipeline. -
Package Changes: Proper version pinning to prevent dependency drift while keeping the migration focused on tsgo.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (fixes existing test issue)
- Code clarity - Good
Note on release.yml
The Greptile analysis raised a valid concern about the release workflow structure. However, this is a pre-existing issue - all the explicit if: conditions without success() were already present before this PR. The PR only adds bun run typecheck to the existing step. While this structural issue is worth addressing, it falls outside the scope of this review which focuses on NEW code.
Automated review by Claude AI
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
This PR upgrades the repo's explicit TypeScript checker to the latest official
tsgopreview and wirestsgointo the project's explicit dev/build/workflow gates.Official basis used during this migration:
@typescript/native-preview@7.0.0-dev.20260321.1microsoft/typescript-goREADME statesnpx tsgoshould be used liketscnoUncheckedSideEffectImportswere used to resolve the newTS2882CSS side-effect import regression exposed by latesttsgoWhat Changed
@typescript/native-previewfrom7.0.0-dev.20251219.1to7.0.0-dev.20260321.1bun run devto runtsgo -p tsconfig.json --noEmitbeforenext devbun run buildto runtsgo -p tsconfig.json --noEmitbeforenext buildtypecheck:tscscript entry so the repo's explicit typecheck path is nowtsgobun run typechecktodev.ymlandrelease.ymlsrc/types/css.d.tsand includedsrc/**/*.d.tsintsconfig.jsonto fix theTS2882CSS side-effect import regression from latesttsgodata/postgres-devanddata/redis-devto.dockerignoreso local Docker builds do not fail on root-owned runtime volume datatests/unit/app/v1/url.test.ts(logger.info is not a function) sobun run test/bun run test:cicomplete cleanlyrelease.ymlso a failedtypecheckcannot continue into commit / tag / image-push stepsnext,@tanstack/react-query, and@biomejs/biome, then revalidated with the currently resolved versions on this branch:next@16.2.1@tanstack/react-query@5.94.5@biomejs/biome@2.4.8Before / After Timings
1. Direct Typecheck Comparison
bun run typecheck(tsgo, cold)12.344s6.825s-5.519s(-44.7%)bun run typecheck(tsgo, hot)12.344s5.501s-6.843s(-55.4%)tsc -p tsconfig.json --noEmit(control)32.178s33.648s+1.470s(+4.6%)Control note:
tsgovstsc: about2.61xfastertsgovs currenttsc:4.93xfaster6.12xfaster2. Fine-Grained Compiler Metrics (
--extendedDiagnostics)tsgo)tsgo)1.038s6.204s+5.166s(+497.7%)0.565s0.535s-0.030s(-5.3%)0.057s0.055s-0.002s(-3.5%)0.361s0.738s+0.377s(+104.4%)603861K1444221K+840360K(+139.2%)Reference control (
tsc --extendedDiagnostics):elapsed=32.320sTotal time: 31.42s3. Dev / Build / Runtime Path Measurements
bun run build58.104s64.134s+6.030s(+10.4%)bun run devuntil/api/versionresponds6750ms4046ms-2704ms(-40.1%)next devinternal ready log5.3s338ms-4.962s(-93.6%)bun run startuntil/api/versionresponds2468msbun run test:e2e3.393sdocker build -f deploy/Dockerfile .95.594sdocker compose --profile app up -d --build appuntil/api/versionresponds100082msInterpretation:
tsgostep is still materially faster thantscbuildis slower overall because it now includes explicittsgopreflight beforenext buildtsgo --extendedDiagnosticsprofile is heavier than the older preview snapshot even though it still beatstscin wall-clock command timeValidation Run
Passed on this branch:
bun run typecheckbun run lintbun run lint:fixbun run formatbun run format:checkbun run buildbun run testbun run test:cibun run test:e2ebun run validate:migrationsbun run i18n:audit-messages-no-emoji:failbun run dev(boot verified locally)bun run start(boot verified locally)docker build -f deploy/Dockerfile -t claude-code-hub-tsgo-prcheck:local .docker compose -f dev/docker-compose.yaml -p cch-dev --profile app up -d --build appKnown baseline failures that were reproduced before and after this migration and are not introduced by this PR:
bun run test:integrationbun run i18n:audit-placeholders:failError Fixes / Classification
A.
tsgo-triggered issues fixed in this PRtsgoraisedTS2882for side-effect CSS imports:src/app/[locale]/layout.tsx->import "../globals.css"src/app/global-error.tsx->import "./globals.css"tsgofollows stricter side-effect import checkingsrc/types/css.d.tswithdeclare module "*.css";and includedsrc/**/*.d.tsintsconfig.jsonB. Project / toolchain issues fixed while doing the migration
release.ymlhad a real gate bug:if:expressions withoutsuccess()typecheckcould still allow commit / tag / image-push steps to continue&& success()to downstream gated release stepsbun run test/bun run test:cihad an existing unit-test mock problem:tests/unit/app/v1/url.test.tsmocked@/lib/loggerwithoutinfo/warnlogger.info, causingTypeError: logger.info is not a functioninfoandwarnLocal Docker build context could fail on repo-local runtime data volumes:
data/postgres-devdata/redis-dev.dockerignoreC. Existing project baseline issues not fixed in this PR
test:integrationbaseline failures:i18n:audit-placeholders:failbaseline failures:Notes
tsgomigration and that thetsgo-related regressions uncovered during the switch were fixed.bun install --frozen-lockfileclean-room reproduction for every workflow job.