Skip to content

fix(admin): preserve translation content when switching between locales#887

Merged
ascorbic merged 2 commits intomainfrom
fix/translation-switch-stale-editor
May 1, 2026
Merged

fix(admin): preserve translation content when switching between locales#887
ascorbic merged 2 commits intomainfrom
fix/translation-switch-stale-editor

Conversation

@ascorbic
Copy link
Copy Markdown
Collaborator

@ascorbic ascorbic commented May 1, 2026

What does this PR do?

Fixes a bug where switching between translations of the same content in the admin editor (e.g. clicking "Edit" on a French translation while viewing the English version) would leave the previous locale's body in the Portable Text editor. Worse: any subsequent edit silently overwrote the new translation's content in formData, corrupting it on autosave.

Root cause (two layers):

  1. PortableTextEditor freezes its initial content via useMemo(() => ..., []) on mount and has no effect to reconcile incoming value changes. This is intentional — re-creating the TipTap instance on every render caused infinite loops with the Suggestion plugin.
  2. ContentEditor reused the same <FieldRenderer> instance on translation switch (key was just name), and the formData reset for the new item happened in a post-render useEffect — one render behind, so even if a remount were forced it would capture stale data.

Translations switch via TanStack Router <Link> to /content/$collection/$id with a different id. The route component stays mounted; only the item prop changes.

Fix (two parts in ContentEditor.tsx):

  • Added React's "store info from previous renders" idiom: a previousItemId state gate that synchronously resets formData, slug, slugTouched, status, internalBylines, lastSavedData, and clears pendingAutosaveStateRef when item.id changes during render. Including lastSavedData in the synchronous reset keeps isDirty stable through the switch (no Saved -> Save -> Saved SaveButton flicker).
  • Keyed <FieldRenderer> by `${name}:${item?.id ?? "new"}` so all field editors remount cleanly on item id change. Sidesteps the deliberate `useMemo([])` memoization in PortableTextEditor without unwinding it.

Closes #

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • `pnpm typecheck` passes
  • `pnpm lint` passes
  • `pnpm test` passes (or targeted tests for my change)
  • `pnpm format` has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include `messages.po` changes except in translation PRs — a workflow extracts catalogs on merge to `main`.
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/...

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 4.7 (OpenCode)

Screenshots / test output

TDD: wrote a failing test first that mocks `PortableTextEditor` to mirror the real freeze behaviour (captures initial `value` once via `useState` initializer, never reconciles), then renders a wrapper that swaps `item` between two locales without unmounting `ContentEditor`. The test asserts both that the displayed content updates and that the editor remounts (`mountCount >= 2`). Verified empirically that reverting either half of the fix (the synchronous reset OR the `key={item.id}` change) makes the test fail — both halves are required.

Adversarial review pass: ran the `adversarial-reviewer` agent against the fix; no critical or high-severity findings. Three medium/low items raised — applied the in-scope ones (avoid `isDirty` flicker by including `lastSavedData` in the synchronous reset; reset mock mount counter in `beforeEach`). Skipped one out-of-scope latent issue (stale `portableTextEditor` ref across remount window — pre-existing, separate fix).

```
Test Files 60 passed (60)
Tests 839 passed (839)
```

ContentEditor stays mounted during TanStack Router param-only navigation
between translations, but PortableTextEditor freezes its initial content
via useMemo([]) on mount, so the editor showed the previous locale's
body after switching. Subsequent edits silently overwrote the new
translation's formData on autosave.

Fix: synchronously reset form state during render when item.id changes
(React 'store info from previous renders' idiom), and key FieldRenderer
by item.id so all field editors remount cleanly on the switch.
Copilot AI review requested due to automatic review settings May 1, 2026 13:19
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 1, 2026

🦋 Changeset detected

Latest commit: fb7df2e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@emdash-cms/admin Patch
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
create-emdash Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

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

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-perf-coordinator fb7df2e May 01 2026, 02:21 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-i18n fb7df2e May 01 2026, 02:21 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs fb7df2e May 01 2026, 02:22 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-playground fb7df2e May 01 2026, 02:22 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-cache fb7df2e May 01 2026, 02:23 PM

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 1, 2026

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@887

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@887

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@887

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@887

emdash

npm i https://pkg.pr.new/emdash@887

create-emdash

npm i https://pkg.pr.new/create-emdash@887

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@887

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@887

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@887

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@887

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@887

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@887

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@887

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@887

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@887

commit: fb7df2e

@ascorbic
Copy link
Copy Markdown
Collaborator Author

ascorbic commented May 1, 2026

/review

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 1, 2026

My adversarial review summary:

Trace of the fix:

  1. Synchronous render-time gate using previousItemId correctly resets all state before child render (canonical React pattern).
  2. key={fieldKey} with item.id forces remount of FieldRenderer (and thus PortableTextEditor), bypassing its useMemo([]) freeze.
  3. The post-render useEffect at line 313 still exists and will redundantly fire (deps include itemDataString) — harmless, just duplicate setState calls that no-op.
  4. lastSavedData is included in the synchronous reset — prevents SaveButton flicker. Good.
  5. pendingAutosaveStateRef.current = null clears stale autosave state. Good.
  6. The autosave effect deps include item?.id, so cleanup fires and clears the timeout when item switches.

Edge cases checked:

  • item undefined → defined: works (gate fires)
  • item defined → null → defined (same id): no reset, preserves edits ✓
  • StrictMode: not enabled in tests, but in production the double-render of state-setter-during-render is the recommended pattern
  • In-flight save during switch: cleanup of autosave timer fires; pending autosave nulled
  • key={${name}:${item?.id ?? "new"}}: when isNew=true and item is undefined, key="${name}:new" which is stable across renders (no spurious remounts on field changes)

One minor concern: the post-render useEffect at line 313 now fires redundantly (the synchronous gate already did the same setState calls). This means when item.id changes, both run. Net effect is identical (idempotent setState with same values), but it's wasted work. Not a bug, not blocking.

Verdict: The fix is correct, well-tested, the TDD approach is sound (test fails when either half of fix is reverted, per PR description), and the changeset is appropriate. I see no bugs.

LGTM!

github run

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an admin editor bug where switching between translations (different item.id on the same mounted route) left Portable Text content stale and could corrupt the newly selected translation’s formData on subsequent edits/autosave.

Changes:

  • Reset editor/form state synchronously when item.id changes to avoid a one-render lag during translation switches.
  • Key field editors by name:itemId to force clean remounts (sidestepping PortableTextEditor’s intentional “freeze initial value on mount” behavior).
  • Add a regression test covering translation/item switching and include a changeset for @emdash-cms/admin.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/admin/src/components/ContentEditor.tsx Synchronously resets state on item.id change and keys FieldRenderer to force remount on translation switch.
packages/admin/tests/components/ContentEditor.test.tsx Adds a targeted regression test using a PortableTextEditor mock that freezes initial value to reproduce the bug.
.changeset/fix-translation-switch-stale-editor.md Patch changeset documenting the stale Portable Text content fix on translation switch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bylines: nextBylines,
}),
);
pendingAutosaveStateRef.current = null;
Comment on lines +277 to +281
// Synchronously reset form state when the underlying item changes (e.g. a
// translation switch where TanStack Router keeps ContentEditor mounted but
// swaps `item` for a different id). The post-render useEffect below also
// syncs item -> formData, but it runs *after* the first render with the new
// item, leaving children (notably PortableTextEditor, which freezes its
Without this cleanup, ContentEditor's portableTextEditor state slot keeps
pointing at a destroyed TipTap instance during the brief remount window
when switching translations (FieldRenderer is re-keyed by item.id), so
DocumentOutline briefly renders against a destroyed editor.

PortableTextEditor's onEditorReady contract now widens to (Editor | null)
and the effect returns a cleanup that calls onEditorReady(null), letting
consumers that pass a setState dispatcher (like setPortableTextEditor)
clear the slot before the next instance mounts.

Verified by TDD against the real component: the new test in
PortableTextEditor.test.tsx fails when the cleanup is removed.
@github-actions github-actions Bot added size/L and removed size/M labels May 1, 2026
@ascorbic ascorbic merged commit 254a443 into main May 1, 2026
36 checks passed
@ascorbic ascorbic deleted the fix/translation-switch-stale-editor branch May 1, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants