fix(admin): sync slash menu state ref synchronously to avoid stale reads#1004
Merged
Conversation
TipTap's Suggestion plugin reads slashMenuStateRef.current synchronously inside onKeyDown. The previous useEffect-based sync runs after commit, so keyboard handlers saw stale state (empty items, null range, stale selectedIndex) on the first event after a state change. That produced five intermittent CI failures in tests/editor/slash-menu.test.tsx where Enter would not execute the selected command and arrow navigation would skip selections on slower runs. Wrap setSlashMenuState with a synchronous setter that writes slashMenuStateRef.current before scheduling the React update, so getState() always returns the most recent intent. Also wraps the 'highlights first item' test in vi.waitFor with an explicit 3s timeout, matching the surrounding selection-state tests. Original diagnosis and fix from #974 by @ahliweb; this is a focused re-application with the scope-creep smoke-test commit dropped and the reviewer feedback from #974 applied (clearer naming, invariant documented, StrictMode note, explicit waitFor timeout).
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-perf-coordinator | 1d0030f | May 12 2026, 03:37 PM |
🦋 Changeset detectedLatest commit: 1d0030f The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-i18n | 1d0030f | May 12 2026, 03:37 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 1d0030f | May 12 2026, 03:38 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 1d0030f | May 12 2026, 03:39 PM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 1d0030f | May 12 2026, 03:41 PM |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate a stale-ref race in the admin PortableTextEditor slash-command menu by synchronizing the slash menu state ref in a way that TipTap’s synchronous onKeyDown handlers can read reliably, and by making the flaky slash-menu browser tests deterministic.
Changes:
- Replace the
useEffect-basedslashMenuStateRefsynchronization with a wrapped setter intended to keep the ref current for synchronous keyboard reads. - Adjust the slash-menu test to wait for the DOM-selected state with an explicit
vi.waitFor({ timeout: 3000 }). - Add a changeset documenting the fix and bumping release versions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/admin/src/components/PortableTextEditor.tsx | Introduces a wrapped setSlashMenuState intended to keep slashMenuStateRef in sync for TipTap Suggestion keyboard handling. |
| packages/admin/tests/editor/slash-menu.test.tsx | Updates the “highlights the first item by default” test to use vi.waitFor to reduce timing flakiness. |
| .changeset/fix-slash-menu-stale-ref.md | Adds release notes/bumps for the stale ref race fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…l updates Address Copilot review on #1004: the previous implementation wrote slashMenuStateRef.current inside the functional updater passed to setSlashMenuStateRaw, which means the ref was only updated when React later executed the updater -- not synchronously at the call site. A TipTap onKeyDown handler firing synchronously after onStateChange could still read stale items/range/selectedIndex via getState(). Compute next from slashMenuStateRef.current immediately, assign the ref, then enqueue a value-based update. This makes the ref the canonical 'latest intent' for any synchronous reader between setter call and React commit. As a side effect, the StrictMode double-invoke note no longer applies (we no longer pass a function to setSlashMenuStateRaw).
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.
What does this PR do?
Fixes a stale ref race in the slash command menu's keyboard handlers, restoring the
tests/editor/slash-menu.test.tsxsuite to deterministic green. This is the only race actually causing the recurring "Browser Tests" failures we have been seeing on main and on every PR that re-runs Browser Tests (e.g. #1000).Root cause
slashMenuStateRefis read synchronously insideonKeyDownhandlers passed to TipTap's Suggestion plugin. The previous code synced the ref viaReact.useEffect, which runs after commit -- so the first keyboard event after a state change saw stale state (emptyitems, nullrange, staleselectedIndex). On slower CI machines this fired reliably, producing five failing tests:highlights the first item by defaultmoves selection down with ArrowDownmoves selection up with ArrowUp from second itemwraps selection around when pressing ArrowUp from first itemexecutes selected command on Enter and converts to headingFix
Wrap
setSlashMenuStatewith a synchronous setter that writesslashMenuStateRef.currentbefore scheduling the React update. The functional-updater branch writes the ref inside the updater so it stays consistent if React replays the updater (StrictMode), and the value branch writes the ref unconditionally before the setter call.getState()now always returns the most recent intent, which is the contract TipTap's Suggestion plugin relies on.Test change
Wrap the "highlights first item" assertion in
vi.waitFor({ timeout: 3000 }), matching the surrounding selection-state tests. The other arrow/Enter tests already used this pattern; this one was the odd one out.Background
This is a focused re-application of #974 (closed by the author who believed the fix had already landed via translation work -- it had not; the
useEffectsync was still on main, as you can verify inPortableTextEditor.tsx). Reviewer feedback from #974 has been applied:_setSlashMenuStatetosetSlashMenuStateRaw(per @ascorbic, clearer than underscore-prefix-as-unused).ref reflects intent, not committed state) and the StrictMode idempotency note (per @ask-bonk).{ timeout: 3000 }(per @ask-bonk).site-matrix-smoke.test.tschanges from the original PR.Credit to @ahliweb for the original diagnosis and fix.
Closes #974
Type of change
Checklist
pnpm typecheckpassespnpm lintpasses (no new diagnostics; baseline 41 == post-change 41)pnpm testpasses (3 consecutive clean runs of the slash-menu suite locally; 23/23)pnpm formathas been runmessages.pochanges except in translation PRs -- a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Local runs (3/3 clean):
```
=== Run 1 ===
Tests 23 passed (23)
=== Run 2 ===
Tests 23 passed (23)
=== Run 3 ===
Tests 23 passed (23)
```