fix(media): re-enable React Compiler for the asset details form - #1841
Draft
pedrobonamin wants to merge 3 commits into
Draft
fix(media): re-enable React Compiler for the asset details form#1841pedrobonamin wants to merge 3 commits into
pedrobonamin wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 1bd3dd6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
✅ E2E Tests🟢 26 passed • view full report • view run Studio: https://plugins-e2e-test-studio-27j1ckgza.sanity.dev Datasets: |
…React Compiler A plain reset() empties react-hook-form's internal field registry and relies on register() re-running on the next render to re-register every field. React Compiler memoizes the registered-field JSX (keyed on the stable register reference), so after the reset that runs when the asset edit dialog opens, string fields stayed unregistered: typing updated the DOM but never marked the form dirty and Save stayed disabled. Passing keepFieldsRef keeps fields registered while still applying the new values. Also convert inline Footer/Header dialog components to plain JSX so the footer subtree (including the Save button) is not remounted whenever captured form state changes, and drop the react/react-compiler lint suppressions that pattern required.
The published build compiles sanity-plugin-media with React Compiler (reactCompiler: true in tsdown.config.ts), but tests ran against uncompiled source, so compiler-specific regressions could not be caught. Apply the same @rolldown/plugin-babel + reactCompilerPreset combination in the plugin's Vitest config and add a regression test asserting the Save button enables after editing the title field.
Remove the temporary 'use no memo' opt-out from Details. The root cause is fixed by keepFieldsRef on reset(): fields stay registered even though the compiler memoizes the register() field JSX, so string-field edits keep marking the form dirty.
cursor
Bot
force-pushed
the
cursor/media-react-compiler-fix-1d31
branch
from
August 5, 2026 14:57
b32601f to
1bd3dd6
Compare
6 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #1840, rebased on
main: removes the temporary'use no memo'compiler opt-out fromDetailsand fixes the Save-button bug at its actual root cause, so the plugin ships fully compiled again.Root cause
DialogAssetEditcallsreset(generateDefaultValues(...))in an effect when the dialog mounts and again whenever the asset is updated elsewhere (mutation listener). react-hook-form'sreset()empties its internal field registry (_fields = {}) and relies on the render-timeregister()calls re-running on the next render to re-register every field. React Compiler memoizes the registered-field JSX inDetailskeyed on the stableregisterreference and asset-derived values, soregister()never re-runs after a reset. From then on, typing in filename / title / alt text / description updates the DOM (the inputs are uncontrolled) but react-hook-form ignores the events —isDirtynever flips and Save stays disabled. Tags kept working because they useController, which doesn't depend on render-timeregister()re-invocation.Fix
{keepFieldsRef: true}to bothreset()call sites (DialogAssetEdit,DialogTagEdit). This react-hook-form option keeps fields registered and applies the new values viasetValueinstead of wiping the registry.'use no memo'fromDetails— with the reset fixed, the compiler-memoized form works correctly.const Footer = () => …components (andHeaderinDialogConfirm) to plain JSX, matching what fix(media): enable Save when editing asset string fields #1840 already did forDialogAssetEdit. An inline component gets a new function identity whenever captured values change, so React unmounted and remounted the whole footer subtree — including the Save button — on every form-state change. This also removes all thereact/react-compilerlint suppressions that the pattern required.Making the regression testable
Vitest previously ran against uncompiled source, so no test could catch compiler-specific regressions (the plugin ships compiled —
reactCompiler: trueintsdown.config.ts). The plugin's Vitest config now applies the same@rolldown/plugin-babel+reactCompilerPresetcombination that the build uses, so the whole suite — including the regression test added in #1840 — exercises compiler output.Counterfactual check: removing
'use no memo'withoutkeepFieldsReffails 3 tests under the compiler ("enables Save after editing a string field (title)", "dispatches asset update when a field changes and the form is submitted", "persists a cleared Description as empty string so EXIF cannot refill it"); withkeepFieldsRefall 205 pass. This also shows theuseFormState/ explicitonChangethreading from #1840 were not the load-bearing parts of that workaround — only the compiler opt-out was.Test plan
pnpm --filter sanity-plugin-media exec vitest run— 205 tests pass, now exercising compiler output with no'use no memo'keepFieldsRefis removedpnpm format,pnpm lint,pnpm knip,pnpm build,pnpm test run(1281 tests) all passdev/test-studio(pnpm dev, compiler enabled viareactCompiler: {}insanity.cli.ts), reproducing the reset-triggered failure mode in a real browser by patching the asset via the API while the dialog is open:keepFieldsRef(compiler on, no'use no memo'): after the externally-triggered form reset, typing in Title/Alt Text updates the inputs but Save stays disabled — bug reproduced.Bug reproduction (broken build — Save stays disabled after the externally-triggered reset):
bug_repro_save_stays_disabled_without_fix.mp4
Fix demo (this branch — Save re-enables after the same reset and the edit saves):
fix_demo_save_enables_after_external_update.mp4
To show artifacts inline, enable in settings.