Skip to content

refactor(workbench): drop reactRefreshHost plumbing#1109

Merged
gu-stav merged 5 commits into
feat/workbenchfrom
SDK-1427
May 28, 2026
Merged

refactor(workbench): drop reactRefreshHost plumbing#1109
gu-stav merged 5 commits into
feat/workbenchfrom
SDK-1427

Conversation

@gu-stav
Copy link
Copy Markdown
Member

@gu-stav gu-stav commented May 20, 2026

Description

With dev.remoteHmr: true landing in @module-federation/vite (via workbench PR sanity-io/workbench#208), the federation plugin wires Fast Refresh through the host automatically. The workbench port no longer needs to leak from devAction down through start{Studio,App}DevServerstartDevServergetViteConfig into @vitejs/plugin-react.

Also drops the .__mf__temp ignore entries from templates and fixtures — federation no longer writes that directory at build time, so the entries are stale.

Net: the build pipeline has zero awareness of whether a workbench host exists.

Depends on a @sanity/federation release containing sanity-io/workbench#208 before merging.


Note

Medium Risk
Touches federated dev HMR wiring and bumps @sanity/federation; behavior depends on the new federation release being present before merge.

Overview
Removes reactRefreshHost from the dev/build Vite path (devActionstartStudioDevServer / startAppDevServerstartDevServergetViteConfig), so @vitejs/plugin-react is no longer pointed at the workbench URL for federated HMR—that behavior is expected from @sanity/federation 0.1.0-alpha.8 and @module-federation/vite@1.16.0 (dev.remoteHmr).

Cleans up stale module-federation ignores: drops .__mf__temp/ from shared gitignore templates, the federated fixture, and related init/bootstrap tests; removes the old changeset that only documented that ignore.

Tests / lockfile: deletes reactRefreshHost assertions; federated build test sets MFE_VITE_NO_TEST_ENV_CHECK so federation plugins still run under Vitest; pnpm-lock.yaml and workspace catalog pin the new federation stack.

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

@gu-stav gu-stav requested a review from a team as a code owner May 20, 2026 07:07
@gu-stav gu-stav requested a review from joshuaellis May 20, 2026 07:07
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

📦 Bundle Stats — @sanity/cli

Compared against feat/workbench (e6795c31)

@sanity/cli

Metric Value vs feat/workbench (e6795c3)
Internal (raw) 2.1 KB -
Internal (gzip) 799 B -
Bundled (raw) 10.97 MB -
Bundled (gzip) 2.06 MB -
Import time 831ms -12ms, -1.4%

bin:sanity

Metric Value vs feat/workbench (e6795c3)
Internal (raw) 1023 B -
Internal (gzip) 486 B -
Bundled (raw) 9.84 MB -
Bundled (gzip) 1.77 MB -
Import time 1.97s +8ms, +0.4%

🗺️ 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 feat/workbench (e6795c31)

Metric Value vs feat/workbench (e6795c3)
Internal (raw) 98.8 KB -
Internal (gzip) 23.3 KB -
Bundled (raw) 21.64 MB -
Bundled (gzip) 3.43 MB -
Import time 797ms +5ms, +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 feat/workbench (e6795c31)

Metric Value vs feat/workbench (e6795c3)
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/src/actions/dev/devAction.ts Outdated
@gu-stav gu-stav changed the title refactor(workbench): drop reactRefreshHost plumbing now that federation owns HMR refactor(workbench): drop reactRefreshHost plumbing May 20, 2026
@gu-stav gu-stav changed the title refactor(workbench): drop reactRefreshHost plumbing refactor(workbench): drop reactRefreshHost plumbing May 20, 2026
@claude
Copy link
Copy Markdown

claude Bot commented May 20, 2026

Claude finished @gu-stav's task in 2m 33s —— View job


PR Review

  • Read PR diff against origin/feat/workbench
  • Review key changed files
  • Verify cursor bugbot concerns
  • Check changeset requirements
  • Post review

The diff is a clean removal: reactRefreshHost plumbing is gone from devActionstartAppDevServer/startStudioDevServerstartDevServergetViteConfig, with all corresponding tests dropped, the federation dep bumped to 0.1.0-alpha.8, and the catalog gated via pnpm-workspace.yaml. grep confirms no leftover reactRefreshHost or __mf__temp references in packages/ or fixtures/.

The three Cursor Bugbot comments on earlier commits look stale after the latest commits:

  1. basePath omitted from dev URL — the URL printed in devAction.ts:72 is the workbench URL, not the studio's. The studio URL with basePath is still emitted by startStudioDevServer.ts:111 and only when workbenchAvailable is false. No regression here, this matches the behavior already on feat/workbench.
  2. Command tests expect old logs — both App dev server started on port (startAppDevServer.ts:41) and running at (startStudioDevServer.ts:121) are still emitted by the source. commands/__tests__/dev.test.ts assertions will still match.
  3. Federated Fast Refresh loss — addressed by the @sanity/federation@0.1.0-alpha.8 bump in package.json, as called out in the PR description.

Issues / nitpicks

  • fixtures/federated-studio/.gitignore is missing a trailing newline (pre-existing — the diff context shows \ No newline at end of file on both sides). Worth fixing while touching the file: replace *.local with *.local\n at end.
  • bootstrapLocalTemplate.test.ts:128 lost its expect(gitignore).toContain('.__mf__temp/') assertion but doesn't replace it with a negative assertion. Consider adding expect(gitignore).not.toContain('.__mf__temp') to guard against a future regression where the entry sneaks back into the shared template.
  • The new vi.stubEnv('MFE_VITE_NO_TEST_ENV_CHECK', 'true') in build.studio.test.ts:96 works, but vi.stubEnv cleanup depends on unstubEnvs: true in vitest config or an explicit vi.unstubAllEnvs() in afterEach. Worth confirming the package's vitest.config.ts resets stubbed env vars between tests so this doesn't leak into other federated-build tests in the same file.

Changeset

The deletion of .changeset/pr-1028.md is correct — that entry described adding .__mf__temp to .gitignore, which this PR reverses. No new changeset is required: the PR targets the feat/workbench feature branch and is an internal refactor with no user-facing behavior change beyond the federation dep bump (which presumably belongs to the feature-branch changeset).

Caveat

Per the PR description, this depends on @sanity/federation@0.1.0-alpha.8 actually shipping dev.remoteHmr: true via @module-federation/vite. I haven't verified that release contents — make sure it's published before merging or Fast Refresh in federated dev will silently break.
• Branch: SDK-1427

Comment thread packages/@sanity/cli/src/actions/dev/startStudioDevServer.ts Outdated
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 05b0145. Configure here.

Comment thread packages/@sanity/cli/src/actions/build/getViteConfig.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

Coverage Delta

File Statements
packages/@sanity/cli-core/src/config/cli/getCliConfig.ts 100.0% (±0%)
packages/@sanity/cli-core/src/config/cli/schemas.ts 100.0% (±0%)
packages/@sanity/cli-core/src/services/cliUserConfig.ts 100.0% (±0%)
packages/@sanity/cli-core/src/util/getSanityConfigDir.ts 66.7% (new)
packages/@sanity/cli/src/actions/build/buildApp.ts 95.3% (±0%)
packages/@sanity/cli/src/actions/build/buildStaticFiles.ts 97.6% (+ 1.0%)
packages/@sanity/cli/src/actions/build/buildStudio.ts 96.7% (±0%)
packages/@sanity/cli/src/actions/build/getViteConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/build/writeSanityRuntime.ts 96.2% (+ 0.7%)
packages/@sanity/cli/src/actions/deploy/deployApp.ts 80.5% (±0%)
packages/@sanity/cli/src/actions/deploy/deployStudioSchemasAndManifests.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/dev/devAction.ts 96.7% (- 3.3%)
packages/@sanity/cli/src/actions/dev/devServerRegistry.ts 93.8% (new)
packages/@sanity/cli/src/actions/dev/extractDevServerManifest.ts 20.0% (new)
packages/@sanity/cli/src/actions/dev/getDevServerConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/dev/startAppDevServer.ts 100.0% (+ 13.0%)
packages/@sanity/cli/src/actions/dev/startDevManifestWatcher.ts 90.9% (new)
packages/@sanity/cli/src/actions/dev/startFederationRegistration.ts 100.0% (new)
packages/@sanity/cli/src/actions/dev/startStudioDevServer.ts 100.0% (+ 5.0%)
packages/@sanity/cli/src/actions/dev/startWorkbenchDevServer.ts 98.7% (new)
packages/@sanity/cli/src/actions/dev/writeWorkbenchRuntime.ts 100.0% (new)
packages/@sanity/cli/src/actions/init/bootstrapLocalTemplate.ts 89.7% (±0%)
packages/@sanity/cli/src/actions/init/bootstrapTemplate.ts 0.0% (±0%)
packages/@sanity/cli/src/actions/init/createAppCliConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/init/createCliConfig.ts 100.0% (+ 50.0%)
packages/@sanity/cli/src/actions/init/createStudioConfig.ts 83.3% (±0%)
packages/@sanity/cli/src/actions/init/initAction.ts 97.2% (+ 0.1%)
packages/@sanity/cli/src/actions/init/initApp.ts 96.0% (±0%)
packages/@sanity/cli/src/actions/init/initStudio.ts 87.0% (±0%)
packages/@sanity/cli/src/actions/init/scaffoldTemplate.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/init/types.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/manifest/extractCoreAppManifest.ts 93.1% (new)
packages/@sanity/cli/src/actions/manifest/extractManifest.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/manifest/types.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/manifest/writeManifestFile.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/dev.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/init.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/manifest/extract.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/schemas/deploy.ts 95.0% (±0%)
packages/@sanity/cli/src/prompts/init/federation.ts 100.0% (new)
packages/@sanity/cli/src/server/devServer.ts 94.1% (±0%)
packages/@sanity/cli/src/util/resolveReactStrictMode.ts 100.0% (new)

Comparing 42 changed files against main @ 96f06469a774b6ec8d21a41de3d0c6cba8f59558

Overall Coverage

Metric Coverage
Statements 84.6% (+ 0.3%)
Branches 74.7% (+ 0.4%)
Functions 84.5% (+ 0.3%)
Lines 85.1% (+ 0.4%)

gu-stav and others added 3 commits May 27, 2026 17:47
…on owns HMR

With `dev.remoteHmr: true` landing in `@module-federation/vite` (via workbench PR
sanity-io/workbench#208), the federation plugin wires Fast Refresh through the host
automatically. The workbench port no longer needs to leak from `devAction` down through
`start{Studio,App}DevServer` → `startDevServer` → `getViteConfig` into `@vitejs/plugin-react`.

Removing that field exposed `workbenchAvailable` as the last piece of workbench-derived state
on `DevActionOptions`, used only to suppress a URL log suffix in the leaf starters. Hoisted
that output into `devAction`, which already owns the "Workbench dev server started at …"
line — all dev-server output formatting now lives in one place.

Net: the build pipeline has zero awareness of whether a workbench host exists.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The federation pipeline no longer creates `.__mf__temp` directories, so
the gitignore entry (and the test asserting it gets scaffolded) are
obsolete. Removes the entry from the template gitignore, the federated
fixture, and the bootstrap test, and drops the changeset that introduced
the entry since the change is no longer shipping.
The reactRefreshHost cleanup also dropped the "App dev server started on port X"
and "...ms and running at http://..." lines from the leaf starters and replaced
them with a single "${label} dev server started at ${appUrl}" line in devAction.
That changed user-visible text and broke dev.test.ts on every shard.

Restore the original log lines (and the workbenchAvailable plumbing they depend
on for the no-workbench branch). The reactRefreshHost removal stays untouched —
build pipeline still has zero awareness of workbench hosts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 28, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​@​sanity/​federation@​0.1.0-alpha.8761008093100

View full report

@gu-stav gu-stav merged commit c492c9f into feat/workbench May 28, 2026
54 checks passed
@gu-stav gu-stav deleted the SDK-1427 branch May 28, 2026 10:04
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