[codex] adopt shared Bazel package workflow#16
Conversation
Greptile SummaryThis PR replaces inline CI and publish workflows with a reusable shared Confidence Score: 4/5Safe to merge for CI verification; the publish path has unresolved P1 concerns from prior rounds that will cause runtime failures. Three prior-round findings remain unresolved: missing
Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub Events
participant CI as ci.yml (Verify)
participant PUB as publish.yml (Publish)
participant TPL as ci-templates/js-bazel-package.yml@dde8c37
participant Bazel as Bazel Build
participant NPM as npm registry
GH->>CI: pull_request / push to main / workflow_dispatch
CI->>TPL: uses (dry_run: true, secrets: inherit)
TPL->>Bazel: bazel build //:pkg //:typecheck //:test
Bazel-->>TPL: artifacts (dist/, dist-types/, typecheck.log)
TPL-->>CI: success / failure (no publish)
GH->>PUB: push tag v*
PUB->>TPL: uses (dry_run: false, secrets: inherit)
TPL->>Bazel: bazel build //:pkg //:typecheck //:test
Bazel-->>TPL: artifacts
TPL->>NPM: npm publish (provenance=true)
NPM-->>TPL: published
TPL-->>PUB: complete
Reviews (7): Last reviewed commit: "chore: repin final shared workflow fixes" | Re-trigger Greptile |
| const packageJson = JSON.parse(read('../package.json')); | ||
| const moduleBazel = read('../MODULE.bazel'); | ||
| const buildBazel = read('../BUILD.bazel'); | ||
| const expectedPnpmVersion = packageJson.packageManager?.replace(/^pnpm@/, ''); |
There was a problem hiding this comment.
expectedPnpmVersion can silently become undefined
If packageManager is absent from package.json, expectedPnpmVersion is undefined. The subsequent actual !== expected comparison then always fails (since any extracted string !== undefined), producing the confusing message: expected "undefined", found "9.15.9". A guard here makes the failure explicit:
| const expectedPnpmVersion = packageJson.packageManager?.replace(/^pnpm@/, ''); | |
| const expectedPnpmVersion = packageJson.packageManager?.replace(/^pnpm@/, ''); | |
| if (!expectedPnpmVersion) { | |
| console.error('packageManager field is missing or not set to a pnpm version in package.json'); | |
| process.exit(1); | |
| } |
| js_run_binary( | ||
| name = "tinyvectors_declarations", | ||
| tool = "//:tsc", | ||
| srcs = glob([ | ||
| "src/**/*.ts", | ||
| ]) + [ | ||
| "package.json", | ||
| "pnpm-lock.yaml", | ||
| "tsconfig.json", | ||
| "tsconfig.declarations.json", | ||
| ":node_modules", | ||
| ], | ||
| args = [ | ||
| "-p", | ||
| "tsconfig.declarations.json", | ||
| ], | ||
| out_dirs = ["dist-types"], | ||
| visibility = ["//visibility:public"], | ||
| ) |
There was a problem hiding this comment.
.svelte files absent from declaration sandbox
src/svelte/index.ts re-exports directly from .svelte files:
export { default as TinyVectors } from './TinyVectors.svelte';
export { default as BlobSVG } from './BlobSVG.svelte';But tinyvectors_declarations.srcs only globs src/**/*.ts, so the .svelte files are not in Bazel's hermetic sandbox when tsc runs. TypeScript falls back to Svelte's ambient declare module '*.svelte' wildcard, which means the emitted dist-types/svelte/index.d.ts types TinyVectors and BlobSVG as the generic ambient component rather than the specific Props interface (with theme, colors, animated, blobCount, etc.). Consumers who import @tummycrypt/tinyvectors/svelte lose all prop-level type checking.
Adding "src/**/*.svelte" to srcs makes the sandbox dependency explicit. Note that plain tsc still cannot parse Svelte syntax to emit prop-typed declarations — fully-typed Svelte component declarations require a Svelte-aware tool (e.g. svelte2tsx or @sveltejs/package). As long as the intent is to rely on the ambient type, the sandbox should at least document that dependency accurately.
| js_run_binary( | |
| name = "tinyvectors_declarations", | |
| tool = "//:tsc", | |
| srcs = glob([ | |
| "src/**/*.ts", | |
| ]) + [ | |
| "package.json", | |
| "pnpm-lock.yaml", | |
| "tsconfig.json", | |
| "tsconfig.declarations.json", | |
| ":node_modules", | |
| ], | |
| args = [ | |
| "-p", | |
| "tsconfig.declarations.json", | |
| ], | |
| out_dirs = ["dist-types"], | |
| visibility = ["//visibility:public"], | |
| ) | |
| js_run_binary( | |
| name = "tinyvectors_declarations", | |
| tool = "//:tsc", | |
| srcs = glob([ | |
| "src/**/*.ts", | |
| "src/**/*.svelte", | |
| ]) + [ | |
| "package.json", | |
| "pnpm-lock.yaml", | |
| "tsconfig.json", | |
| "tsconfig.declarations.json", | |
| ":node_modules", | |
| ], | |
| args = [ | |
| "-p", | |
| "tsconfig.declarations.json", | |
| ], | |
| out_dirs = ["dist-types"], | |
| visibility = ["//visibility:public"], | |
| ) |
Summary
ci-templatescommitWhy
tinyvectorswas carrying duplicated workflow logic for the same package-authority contract now shared across the package repos. This keeps the repo thinner and makes the authority lane easier to reason about.Validation
.github/workflows/ci.yml.github/workflows/publish.yml