[lexical] Bug Fix: Defer onUpdate callbacks during nested commits#8672
[lexical] Bug Fix: Defer onUpdate callbacks during nested commits#8672Rohithmatham12 wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
potatowagon
left a comment
There was a problem hiding this comment.
Reviewed by Navi (Tater Thoughts Bobblehead) on behalf of @potatowagon.
Assessment: LGTM ✅
What this does: Fixes #8359 — when setEditorState() is called inside an active editor.update(), the nested commit was prematurely draining editor._deferred, causing onUpdate callbacks to fire before the outer update function returned. The fix gates triggerDeferredUpdateCallbacks on !previouslyUpdating, ensuring the outer update drains them after its updateFn completes.
What I checked:
- Logic correctness: The
previouslyUpdatingvariable is already captured at the top of$commitPendingUpdates(line ~548) — reusing it here is clean. The no-pending-state early-return path (line ~547) now also checks!editor._updating, which is correct — if an update is active, deferred callbacks should wait. - Edge cases: (a) Normal non-nested commits:
previouslyUpdatingis false → callbacks drain immediately as before. (b) Nested commit viasetEditorState(): callbacks stay queued → outer update drains after returning. (c) Multiple nested commits: each defers; the final outer drain handles all accumulated callbacks. - Test coverage: Comprehensive test verifies the exact ordering:
update start→update end→after update→ (microtask) →onUpdate. Good use ofPromise.resolve()to verify async drain. - Regression risk: Low. The only behavioral change is callback timing for nested commits — callers should not depend on
onUpdatefiring synchronously mid-update. Aligns with documented$onUpdatesemantics. - www compat: No API surface changes. Internal timing fix only. Safe for downstream consumers.
CI: CLA ✅, Vercel ✅. Full test matrix not yet triggered (PR is <10min old) — recommend waiting for unit/browser/e2e green before merge.
Ready to approve once full CI passes.
|
Title and description doesn't match the pull request template |
Description
When
setEditorState()is called from inside an activeeditor.update(), it can force a commit before the outer update callback has returned. That nested commit currently drainseditor._deferred, so the outer update'sonUpdatecallback can run before the update that scheduled it has completed.This keeps deferred callbacks queued while a commit is forced during an existing update. The outer update then drains them after its update function returns, preserving the expected
$onUpdatetiming. The existing no-pending-state fallback now also avoids draining callbacks while an update is active.Closes #8359
Test plan
Before
Calling
setEditorState()from inside an activeeditor.update()could run deferredonUpdatecallbacks before the outer update function returned.After
Nested commits keep deferred callbacks queued until the outer update completes, so
onUpdatetiming stays consistent with the update lifecycle.Automated tests:
/Users/rohithmatam/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node node_modules/vitest/vitest.mjs --project unit packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx --run/Users/rohithmatam/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node node_modules/vitest/vitest.mjs --project unit packages/lexical-react/src/__tests__/unit/LexicalExtensionComposer.test.tsx --run/Users/rohithmatam/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node node_modules/prettier/bin/prettier.cjs --check packages/lexical/src/LexicalUpdates.ts packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx/Users/rohithmatam/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node node_modules/eslint/bin/eslint.js packages/lexical/src/LexicalUpdates.ts packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx