refactor(trpc): migrate declaration emit to tsgo and stop committing dist#1562
Conversation
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (3 files)
Reviewed by gpt-5.4-20260305 · 583,370 tokens |
c1a9e78 to
0a0b131
Compare
7a13ba8 to
2c3cd6d
Compare
0a0b131 to
b33a798
Compare
Pull request was closed
Pull request was closed
81f59e8 to
32d0499
Compare
32d0499 to
2360311
Compare
…dist - Replace tsc with tsgo for the @kilocode/trpc build step for faster declaration emit - Add rootDir to tsconfig.json for tsgo 7.0 compatibility - Update rollup alias resolver with resolveDts() helper to handle tsgo's directory-style index.d.ts output - Stop committing the 16k-line generated dist/index.d.ts to avoid rebase conflicts; gitignore the full packages/trpc/dist/ directory - Extract the prepare lifecycle hook into scripts/prepare.sh with set -euo pipefail; auto-rebuild trpc after pnpm install; skip in CI - Add trpc build step to CI lint job for type-aware oxlint rules - Fix pre-existing workspace test failures exposed by shared_changed: skip workspaces with no test files in changed-workspaces.sh, add created_at to cloudflare-o11y pipeline assertions, add jest.config.mjs for cloudflare-ai-attribution ts-jest transform
2360311 to
8a62d0a
Compare
| [ -n "$has_test" ] || continue | ||
|
|
||
| # Skip workspaces whose test script exists but has no test files | ||
| test_file_count=$(find "$dir" -type f \( -name '*.test.ts' -o -name '*.test.tsx' -o -name '*.test.js' -o -name '*.test.jsx' -o -name '*.spec.ts' -o -name '*.spec.tsx' -o -name '*.spec.js' -o -name '*.spec.jsx' \) -not -path '*/node_modules/*' 2>/dev/null | head -1) |
There was a problem hiding this comment.
WARNING: This pipeline can terminate the script as soon as a workspace has multiple tests
Because the script runs under set -euo pipefail, find ... | head -1 returns a non-zero status when head closes the pipe after the first match. Packages like cloud-agent and kiloclaw already have multiple matching test files, so the matrix generation can exit here instead of returning JSON. Use find ... -print -quit (or add || true) to probe for one match without tripping pipefail.
Summary
@kilocode/trpcdeclaration emit fromtsctotsgofor faster buildsdist/index.d.tsto avoid painful rebase conflicts whenever router types change on main; the fullpackages/trpc/dist/directory is now gitignoredpreparelifecycle hook intoscripts/prepare.shso trpc types are auto-rebuilt afterpnpm install; the script skips both husky and the trpc build in CI since they are unnecessary therepackages/trpcchanges triggeringshared_changedinchanged-workspaces.shKey changes:
packages/trpc/tsconfig.json: added explicitrootDirfor tsgo 7.0 compatibilitypackages/trpc/rollup.config.mjs: updated alias resolver withresolveDts()helper to handle tsgo's directory-styleindex.d.tsoutput (vs tsc's flat.d.tsfiles)scripts/prepare.sh: new file withset -euo pipefailfor robust error handling, CI skip via$CIenv var.github/workflows/ci.yml: addedpnpm --filter @kilocode/trpc run buildstep before lintscripts/changed-workspaces.sh: skip workspaces that have atestscript but no actual test filescloudflare-o11y/test/index.spec.ts: addedcreated_at: expect.any(Number)to pipeline assertionscloudflare-ai-attribution/jest.config.mjs: added missing jest config with ts-jest transformVerification
pnpm install— prepare hook runs successfully, trpc types rebuiltpnpm typecheck— passes (tsgo --noEmit)pnpm --filter cloudflare-o11y test— 61 tests passpnpm --filter cloudflare-ai-attribution test— 22 tests passchanged-workspaces.shno longer includes cloudflare-images-mcp or cloudflare-db-proxy (no test files)Visual Changes
N/A
Reviewer Notes
resolveDts()helper in rollup.config.mjs is needed because tsgo preserves path aliases differently from tsc — it may emitfoo/index.d.tsinstead offoo.d.ts. The helper tries both forms.${CI:-}syntax in prepare.sh is required becauseset -utreats unset variables as errors;$CIis unset in local dev environments.workspace-testsCI matrix was always empty on main pushes (no changed workspaces). This branch triggeredshared_changed=true(viapackages/trpc/**), which runs all workspaces with atestscript.