Skip to content

refactor: split out build logic into a new package#1062

Open
snocorp wants to merge 33 commits into
mainfrom
cli-build-package
Open

refactor: split out build logic into a new package#1062
snocorp wants to merge 33 commits into
mainfrom
cli-build-package

Conversation

@snocorp
Copy link
Copy Markdown
Contributor

@snocorp snocorp commented May 7, 2026

Description

The goal of this PR is to move some logic from the main cli package into a new cli-build package. This will eventually allow the runtime-cli package to use the build logic to prepare blueprints assets to be deployed by a blueprints resource provider. I stopped here to help make these changes easier to digest. The next PR will likely involve moving a lot of files from cli to cli-build and that's a lot of noise whereas this PR mostly focuses on the new project setup.

What to review

Ensure package setup is done correctly and follows standards and conventions for this repo.

Testing

Testing will be done by using pkg-pr to create a build from this PR


Note

Medium Risk
Moderate risk because it introduces a new workspace package and rewires CLI build-time imports/packaging (favicons/static asset generation) plus CI/E2E publishing flows, which could impact build output or release packaging if misconfigured.

Overview
Introduces a new workspace package, @sanity/cli-build, to host shared build utilities (initially copyDir, favicon + web manifest helpers) with its own build/test/lint configs and _internal export surface.

Refactors @sanity/cli build pipeline to consume these utilities via @sanity/cli-build/_internal (including replacing read-package-up root resolution with getDefaultFaviconsPath) and updates related unit tests accordingly.

Updates repo tooling/CI to recognize and ship the new package: workspace + lockfile, check:types/Turbo deps, Knip and Vitest projects/coverage, E2E tarball packing/install, and preview package publishing/docs; also removes @sanity/cli’s bundled static directory and notes bundle-stats TODO for the new package.

Reviewed by Cursor Bugbot for commit 314e9f6. Bugbot is set up for automated code reviews on this repo. Configure here.

@snocorp snocorp self-assigned this May 7, 2026
@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Claude finished @snocorp's task in 5m 6s —— View job


Review

Most prior bugbot comments have been resolved in later commits (static folder is now back in the cli-build files array, typescript devDep added, vitest oclif config cleaned up, e2e tsconfig now correctly maps @sanity/cli-build/_internalsrc/_exports/_internal.ts, the read-package-up dep was dropped from @sanity/cli, and the getDefaultFaviconsPath test now asserts on the returned path).

Remaining issues worth fixing:

1. packages/@sanity/cli-build/.swcrc — unnecessary React transform

The new package has no JSX/React but .swcrc still includes a transform.react block (line 7‑11). It's leftover from copy‑pasting cli-core/cli. Drop the transform field to avoid the misleading impression that this package compiles JSX.

-  "transform": {
-    "react": {
-      "runtime": "automatic"
-    }
-  },
   "target": "es2023"

2. writeWebManifest.ts / generateWebManifest.ts lack dedicated unit tests

Only copyDir.test.ts and writeFavicons.test.ts exist in the new package. writeFavicons.test.ts mocks out ../writeWebManifest.js and ../../util/copyDir.js, so the bodies of writeWebManifest and generateWebManifest aren't exercised by any test in cli-build. The 100% number in the coverage delta is a side effect of istanbul instrumenting the loaded modules; it doesn't reflect actual assertions on their behavior. Add small tests for generateWebManifest(basePath) (verifying the icon URLs/sizes) and writeWebManifest (verifying the file is written to manifest.webmanifest with the expected content).

3. writeWebManifest.ts:12-14 — dead .catch(skipIfExistsError)

fs.writeFile with default flags overwrites and never throws EEXIST, so the .catch(skipIfExistsError) is dead code. This was pre-existing in cli and just moved here, but worth removing while the surface area is small:

await fs.writeFile(path.join(destDir, 'manifest.webmanifest'), content, 'utf8')

Notes (non-blocking)

  • bundle-stats.yml:32 has # TODO add packages/@sanity/cli-build after initial release — intentional and matches the PR description, just flagging it isn't forgotten.
  • The changeset is appropriate: minor bump for the new @sanity/cli-build package, patch for @sanity/cli (build path indirection changed at runtime). Wording is fine for an internal package introduction.
  • cli-build's package.json main/types point to _internal.js, but the exports map only exposes ./_internal. Tools that respect exports will block import '@sanity/cli-build'; tools that fall back to main will succeed. Since the package is marked internal, this is mostly cosmetic, but consider setting main/types to something that signals "do not import from root" or aligning with cli-core's pattern.
  • Agree with @rexxars' note about actions/build/... nomenclature — for a build utility package, flattening to src/<file>.ts would be clearer. Not blocking for this PR since you mentioned a follow-up is coming.
    · branch cli-build-package

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Bundle Stats — An error occurred while calculating bundle sizes.

Error details
From https://github.com/sanity-io/cli
 * branch            cc636d1f9ea30bb6b155d1d575bfac636c4294fb -> FETCH_HEAD
Building baseline...

Attention:
Turborepo now collects completely anonymous telemetry regarding usage.
This information is used to shape the Turborepo roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://turborepo.dev/docs/telemetry

Measuring baseline for packages/@sanity/cli
Found 1 exports and 1 bin entries
Measuring internal sizes...
Bundling @sanity/cli...
"fsevents" is imported by "node_modules/.pnpm/rollup@4.60.2/node_modules/rollup/dist/es/shared/node-entry.js", but could not be resolved – treating it as an external dependency.
Bundling bin:sanity...
Benchmarking import @sanity/cli (1/2)...
Benchmarking import bin:sanity (2/2)...
Measuring baseline for packages/@sanity/cli-build
Error: ENOENT: no such file or directory, open '/home/runner/work/cli/cli/packages/@sanity/cli-build/package.json'

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Coverage Delta

File Statements
packages/@sanity/cli-build/src/actions/build/generateWebManifest.ts 100.0% (new)
packages/@sanity/cli-build/src/actions/build/writeFavicons.ts 100.0% (new)
packages/@sanity/cli-build/src/actions/build/writeWebManifest.ts 100.0% (new)
packages/@sanity/cli-build/src/util/copyDir.ts 100.0% (new)
packages/@sanity/cli/src/actions/build/buildStaticFiles.ts 96.5% (±0%)
packages/@sanity/cli/src/actions/build/getViteConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/server/vite/plugin-sanity-favicons.ts 18.8% (±0%)

Comparing 7 changed files against main @ 423ea6659df51bd7a9f06672df8134957ef29c6b

Overall Coverage

Metric Coverage
Statements 84.3% (+ 0.1%)
Branches 74.2% (+ 0.1%)
Functions 84.1% (+ 0.1%)
Lines 84.7% (+ 0.1%)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

📦 Bundle Stats — @sanity/cli

Compared against main (cc636d1f)

@sanity/cli

Metric Value vs main (cc636d1)
Internal (raw) 2.1 KB -
Internal (gzip) 799 B -
Bundled (raw) 10.97 MB -
Bundled (gzip) 2.06 MB -
Import time 863ms +4ms, +0.4%

bin:sanity

Metric Value vs main (cc636d1)
Internal (raw) 975 B -
Internal (gzip) 460 B -
Bundled (raw) 9.84 MB -
Bundled (gzip) 1.77 MB -
Import time 2.05s +290ms, +16.5% ⚠️

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — @sanity/cli-core

Compared against main (cc636d1f)

Metric Value vs main (cc636d1)
Internal (raw) 95.5 KB -
Internal (gzip) 22.5 KB -
Bundled (raw) 21.60 MB -
Bundled (gzip) 3.42 MB -
Import time 821ms +6ms, +0.7%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — create-sanity

Compared against main (cc636d1f)

Metric Value vs main (cc636d1)
Internal (raw) 976 B -
Internal (gzip) 507 B -
Bundled (raw) 50.7 KB -
Bundled (gzip) 12.6 KB -
Import time ❌ ChildProcess denied: node -
Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

Preview this PR with pkg.pr.new

Run the Sanity CLI

npx https://pkg.pr.new/sanity-io/cli/@sanity/cli@d18dcb5 <command>

...Or upgrade project dependencies

📦 @sanity/cli
pnpm install https://pkg.pr.new/sanity-io/cli/@sanity/cli@d18dcb5ede23284cdadb50db930aa8428a1979bd
📦 @sanity/cli-build
pnpm install https://pkg.pr.new/sanity-io/cli/@sanity/cli-build@d18dcb5ede23284cdadb50db930aa8428a1979bd
📦 @sanity/cli-core
pnpm install https://pkg.pr.new/sanity-io/cli/@sanity/cli-core@d18dcb5ede23284cdadb50db930aa8428a1979bd
📦 @sanity/cli-test
pnpm install https://pkg.pr.new/sanity-io/cli/@sanity/cli-test@d18dcb5ede23284cdadb50db930aa8428a1979bd
📦 @sanity/eslint-config-cli
pnpm install https://pkg.pr.new/sanity-io/cli/@sanity/eslint-config-cli@d18dcb5ede23284cdadb50db930aa8428a1979bd

View Commit (d18dcb5)

@snocorp snocorp marked this pull request as ready for review May 11, 2026 13:48
@snocorp snocorp requested a review from a team as a code owner May 11, 2026 13:48
Comment thread packages/@sanity/cli-build/package.json
Comment thread packages/@sanity/cli/package.json
Comment thread packages/@sanity/cli/package.json
Comment thread packages/@sanity/cli-build/vitest.config.ts Outdated
Comment thread packages/@sanity/cli-build/package.json
Comment thread packages/@sanity/cli-build/tsconfig.json Outdated
Copy link
Copy Markdown
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking but:

  • The actions nomenclature is more of a CLI thing (separating the actual "actions" from the "command", the former being more of the "api" and the latter being what gathers input and controls the flow of the CLI command), so for a build package that is more of an API, these can probably just live at the root of src
  • The public API surface is a little weird right now, but I understand the intention is to move to higher level APIs and expose those

@rexxars rexxars changed the title refactor: Split out build logic into a new package refactor: split out build logic into a new package May 12, 2026
rexxars
rexxars previously approved these changes May 12, 2026
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 125f435. Configure here.

Comment thread packages/@sanity/cli-e2e/tsconfig.json Outdated
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.

2 participants