Skip to content

refactor: Move buildApp and buildStudio logic to cli-build package#1148

Open
snocorp wants to merge 7 commits into
mainfrom
cli-build-actions
Open

refactor: Move buildApp and buildStudio logic to cli-build package#1148
snocorp wants to merge 7 commits into
mainfrom
cli-build-actions

Conversation

@snocorp
Copy link
Copy Markdown
Contributor

@snocorp snocorp commented Jun 1, 2026

Description

This PR finally allows us to build studios and apps from the runtime-cli. There may be a few tweaks needed once we integrate but this refactor covers the initial requirements.

What to review

Ensure I didn't refactor the tests in a way that makes them useless. Had to move a bunch of tests that relied on mocking that was no longer possible.

Testing

Lots of test changes but coverage should remain intact.


Note

Medium Risk
Large refactor of the build and auto-update prompt paths with a breaking change to @sanity/cli-build/_internal/build exports; risk is mitigated by relocated tests and thin CLI adapters, but runtime-cli integration still needs validation.

Overview
Moves studio and app build orchestration from @sanity/cli into @sanity/cli-build, so the same paths can be reused (e.g. from runtime-cli) without going through Oclif flags.

@sanity/cli buildApp / buildStudio are now thin wrappers: they map CLI config and flags into a BuildOptions object and call the internal implementations, including a new viteReactPluginFactory so @vitejs/plugin-react stays on the CLI side while getViteConfig only receives a buildViteReactPlugin callback.

The @sanity/cli-build/_internal/build surface is narrowed (drops several helpers like buildDebug, finalizeViteConfig, telemetry types, etc.) and adds buildApp, buildStudio, checkRequiredDependencies, and compareDependencyVersions (moved from CLI). buildStaticFiles stops re-exporting through _internal and takes injected env vars instead of hard-coded app/studio getters.

Tests for auto-update prompts, version mismatches, and prerelease handling move from CLI command tests into cli-build integration-style tests against buildApp / buildStudio directly; CLI build tests keep command-level coverage. cli-build vitest gets worker globalSetup; dev server imports shared checks from cli-build.

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

@snocorp snocorp self-assigned this Jun 1, 2026
@snocorp snocorp requested a review from a team as a code owner June 1, 2026 17:07
@claude
Copy link
Copy Markdown

claude Bot commented Jun 1, 2026

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


Claude PR Review

  • Read changed files and PR diff
  • Verify changeset
  • Investigate test mocking concerns (Bugbot findings)
  • Check exports / API narrowing for breaking changes
  • Post review feedback

Bugbot findings — status

All three Cursor Bugbot findings have already been addressed in later commits on this branch:

  1. App build tests disabled (e5d42d7) → buildApp.test.ts now contains 9 active tests covering version diff confirm/decline, prerelease prompt cancel/disable, unattended + non-interactive modes, and custom-output cleanup. Coverage is on par with the studio tests.
  2. Test never wires upgrade mock (a380fd2) → at buildStudio.test.ts:277 the test spreads buildOptions(cwd, output) and then explicitly overrides upgradePackages: mockedUpgradePackages, so the not.toHaveBeenCalled() assertion is meaningful.
  3. Mock path uses .ts extension (970cf61) → fixed in ade3f12; buildStudio.appIdWarning.test.ts:34 now mocks '../../../telemetry/build.telemetry.js'.

Issues

1. Dead getAppId mocks in *.appIdWarning.test.ts (M)

cli-build's buildApp.ts and buildStudio.ts no longer call getAppId()appId is now a BuildOptions field. The leftover mocks no longer intercept anything and are misleading:

  • buildApp.appIdWarning.test.ts:6 (mockGetAppId), :39-41 (vi.mock('../../../util/appId.js', …)), and the mockGetAppId.mockReturnValue(…) calls inside each test.
  • buildStudio.appIdWarning.test.ts:8, :42-44, plus calls in each test.

The actual driver of behavior is appId in the spread buildOptions. Recommend removing these mocks so the tests don't suggest a dependency that no longer exists. Fix this →

2. Inconsistent .js extension in test imports (minor)

The two new test files break the repo's NodeNext import convention:

  • buildApp.test.ts:9from '../buildApp'
  • buildStudio.test.ts:9from '../buildStudio'

Neighboring tests (buildApp.appIdWarning.test.ts:4, buildStudio.appIdWarning.test.ts:4, getViteConfig.test.ts:13) all use .js. Add .js to match.

3. Changeset summary is incomplete (minor)

.changeset/nine-steaks-shout.md describes only the export narrowing:

Reduce exported internal build logic to only functions used by the cli package.

But the PR's primary change — described in the title and body — is moving buildApp/buildStudio orchestration into cli-build so it can be invoked from runtime-cli. Consider expanding the summary to capture the new exports (e.g. "Move build orchestration into _internal/build and narrow exports to the actively used surface"). Minor bump is appropriate for a 0.x package adding new exports.

Observations (not blocking)

  • The viteReactPluginFactory is declared () => Plugin[] but @vitejs/plugin-react returns PluginOption[]. Compiles today; flag if TypeScript ever tightens.
  • @vitejs/plugin-react was moved into @sanity/cli's runtime dependencies. Bundle stats show no significant import-time regression, so no changeset for cli is needed.
  • Removed _internal/build exports (buildDebug, buildVendorDependencies, finalizeViteConfig, writeFavicons, AppBuildTrace, StudioBuildTrace, copyDir) are no longer referenced by @sanity/cli — verified via grep. Safe within this monorepo; external integrators on cli-build will need updates (acceptable given _internal/ path and 0.x version).
    · Branch: cli-build-actions

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

📦 Bundle Stats — @sanity/cli

Compared against main (87cd8f26)

@sanity/cli

Metric Value vs main (87cd8f2)
Internal (raw) 2.1 KB -
Internal (gzip) 799 B -
Bundled (raw) 10.97 MB -
Bundled (gzip) 2.06 MB -
Import time 818ms +2ms, +0.2%

bin:sanity

Metric Value vs main (87cd8f2)
Internal (raw) 1023 B -
Internal (gzip) 486 B -
Bundled (raw) 9.87 MB -
Bundled (gzip) 1.77 MB -
Import time 1.92s +21ms, +1.1%

🗺️ 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 (87cd8f26)

Metric Value vs main (87cd8f2)
Internal (raw) 96.3 KB -
Internal (gzip) 22.7 KB -
Bundled (raw) 21.64 MB -
Bundled (gzip) 3.43 MB -
Import time 782ms +1ms, +0.1%

🗺️ 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 (87cd8f26)

Metric Value vs main (87cd8f2)
Internal (raw) 908 B -
Internal (gzip) 483 B -
Bundled (raw) 931 B -
Bundled (gzip) 491 B -
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.

Comment thread packages/@sanity/cli-build/src/actions/build/__tests__/buildApp.test.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Coverage Delta

File Statements
packages/@sanity/cli-build/src/actions/build/buildApp.ts 95.1% (new)
packages/@sanity/cli-build/src/actions/build/buildStaticFiles.ts 96.7% (new)
packages/@sanity/cli-build/src/actions/build/buildStudio.ts 96.5% (new)
packages/@sanity/cli-build/src/actions/build/buildVendorDependencies.ts 67.4% (±0%)
packages/@sanity/cli-build/src/actions/build/checkRequiredDependencies.ts 100.0% (new)
packages/@sanity/cli-build/src/actions/build/getAutoUpdatesImportMap.ts 100.0% (new)
packages/@sanity/cli-build/src/actions/build/getViteConfig.ts 100.0% (±0%)
packages/@sanity/cli-build/src/actions/build/handlePrereleaseVersions.ts 80.0% (new)
packages/@sanity/cli-build/src/util/compareDependencyVersions.ts 100.0% (new)
packages/@sanity/cli-build/src/util/formatSize.ts 100.0% (new)
packages/@sanity/cli-build/src/util/moduleFormatUtils.ts 100.0% (new)
packages/@sanity/cli-build/src/util/warnAboutMissingAppId.ts 100.0% (new)
packages/@sanity/cli/src/actions/build/buildApp.ts 100.0% (+ 4.7%)
packages/@sanity/cli/src/actions/build/buildStudio.ts 100.0% (+ 3.3%)
packages/@sanity/cli/src/actions/build/viteReactPluginFactory.ts 100.0% (new)
packages/@sanity/cli/src/actions/dev/startStudioDevServer.ts 95.0% (±0%)
packages/@sanity/cli/src/server/devServer.ts 94.4% (±0%)

Comparing 17 changed files against main @ 87cd8f26282b6a57501ef78de37b66a2ebf1c26e

Overall Coverage

Metric Coverage
Statements 84.3% (±0%)
Branches 74.3% (- 0.0%)
Functions 84.2% (+ 0.0%)
Lines 84.8% (+ 0.0%)

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 970cf61. Configure here.

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