[lexical] Bug Fix: recover from an empty editor state instead of throwing - #9183
potatowagon wants to merge 1 commit into
Conversation
…wing ## Description `editor.setEditorState()` currently throws an invariant (minified error facebook#38, "setEditorState: the editor state is empty") whenever it is handed an `EditorState` whose root has no children. The check is right that such a state is not the canonical empty document — it reconciles to a contenteditable with no block element in it at all, so there is nowhere to place a caret, and everything downstream that assumes `root.getFirstChild()` exists has to special-case it — but throwing is a harsh response to a shape that routinely arrives from *outside* the editor: content that was persisted while the editor was empty round-trips to `{"root":{"children":[]}}` and comes back through `parseEditorState()` on rehydrate. In production that turns recoverable bad input into a crash of the host application. This change keeps the condition loud where it is actionable and recoverable where it is not: - It is reported through the editor's warn-level hook (`onWarn`), whose default throws in development — so the dev-time behaviour of `setEditorState` is unchanged — and only `console.warn`s in production. Embedders can pass their own `onWarn` to route it to telemetry, or to restore the old throw-everywhere behaviour. - The state is then recovered to the canonical empty document (a root with a single empty paragraph) inside the update that applies it, so the editor that comes out the other side is a normal empty editor. This is the pattern the update-recursion guard in `LexicalUpdates` already uses, and `onWarn` is documented as being for conditions the editor has already recovered from — hence the recovery rather than a bare warning. The warning is raised with a direct `editor._onWarn(...)` call rather than an `invariant`, because `transform-error-messages` rewrites invariant call sites into `formatProdErrorMessage(...)` and the warning would never reach the hook in a built artifact. Error code 38 is left in `scripts/error-codes/codes.json` untouched. No API changes; `setEditorState` keeps its signature and its development-time behaviour. ## Test plan Two tests added to `packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx`: an empty parsed state is recovered to one empty paragraph, is reported once to `onWarn`, renders a block in the DOM and accepts text; and the default `onWarn` still throws in development. ### Before ``` $ npx vitest run --project unit \ packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx -t 'empty editor state' ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯ FAIL |unit| packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx > LexicalEditor tests > setEditorState recovers an empty editor state and reports it to onWarn Error: setEditorState: the editor state is empty. Ensure the editor state's root node never becomes empty. ❯ invariant packages/lexical-internal/src/invariant.ts:28:9 ❯ LexicalEditor.setEditorState packages/lexical/src/LexicalEditor.ts:1737:7 ❯ packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx:3030:12 Test Files 1 failed (1) Tests 1 failed | 1 passed | 92 skipped (94) ``` ### After ``` $ npx vitest run --project unit \ packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx -t 'empty editor state' ✓ |unit| packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx (94 tests | 92 skipped) 156ms Test Files 1 passed (1) Tests 2 passed | 92 skipped (94) $ pnpm run test-unit Test Files 257 passed (257) Tests 5134 passed | 1 skipped (5135) $ pnpm run tsc # clean $ pnpm run flow # No errors! $ pnpm run prettier # no differences $ pnpm run lint # clean ``` E2E and browser-mode tests were not run in this environment (no browser runner available); the change is covered by the unit tests above.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
potatowagon
changed the title
|
Some archaeology on where this invariant came from, since "why was it a throw in the first place?" is the obvious question to ask of this PR. OriginIt arrived in #669 ("Add further invariants", Sep 2021), back when it read The twin invariant was already removed#669 added the check in two places:
The second one was removed in #1316 ("Make root selectable", Feb 2022), by the same author. That is likely the original rationale, and it has expired: before root was selectable, an empty root meant there was nowhere for a selection to live. Once root became selectable that hazard went away, and the update-path check went with it. The The result today is an asymmetry — Lexical produces a state it then refuses to accept: editor.update(() => { $getRoot().clear(); }, {discrete: true});
// does not throw
editor.getEditorState().isEmpty(); // true
JSON.stringify(editor.getEditorState());
// {"root":{"children":[],"direction":null,"format":"","indent":0,"type":"root","version":1}}
// ^ exactly the payload setEditorState rejectsSo This has surfaced repeatedly
What that means for this changeThe condition is worth reporting — a childless root is not the canonical empty document, and code downstream that assumes Risks I can see, for reviewers to weigh:
Happy to gate the recovery behind an opt-in, or to warn without recovering, if either fits the project's intent better. I went with recover-by-default because a childless root reconciles to a contenteditable with no block element in it at all, so a bare warning leaves the editor in a state with nowhere to place a caret. |
| /** | ||
| * Imperatively set the EditorState. Triggers reconciliation like an update. | ||
| * | ||
| * An empty EditorState (a root with no children) is not the canonical empty |
There was a problem hiding this comment.
I don’t think this edge case should be in the documentation of the function
| // bundle, dropping the editor reference, so the warning would never | ||
| // reach `_onWarn` in a built artifact. | ||
| this._onWarn( | ||
| new Error( |
There was a problem hiding this comment.
This long string doesn’t get reduced in the codes map. It’s probably not even worth a warning, especially in prod
There was a problem hiding this comment.
also the whole rationale for using a bare message is moot if the warning is moved to the same location where the workaround occurs (inside if (isEmptyEditorState) { … })
2 participants
Description
editor.setEditorState()currently throws an invariant (minified error #38, "setEditorState: the editor state is empty") whenever it is handed anEditorStatewhose root has no children. The check is right that such a state is not the canonical empty document — it reconciles to a contenteditable with no block element in it at all, so there is nowhere to place a caret, and everything downstream that assumesroot.getFirstChild()exists has to special-case it — but throwing is a harsh response to a shape that routinely arrives from outside the editor: content that was persisted while the editor was empty round-trips to{"root":{"children":[]}}and comes back throughparseEditorState()on rehydrate. In production that turns recoverable bad input into a crash of the host application.This change keeps the condition loud where it is actionable and recoverable where it is not:
onWarn), whose default throws in development — so the dev-time behaviour ofsetEditorStateis unchanged — and onlyconsole.warns in production. Embedders can pass their ownonWarnto route it to telemetry, or to restore the old throw-everywhere behaviour.This is the pattern the update-recursion guard in
LexicalUpdatesalready uses, andonWarnis documented as being for conditions the editor has already recovered from — hence the recovery rather than a bare warning. The warning is raised with a directeditor._onWarn(...)call rather than aninvariant, becausetransform-error-messagesrewrites invariant call sites intoformatProdErrorMessage(...)and the warning would never reach the hook in a built artifact. Error code 38 is left inscripts/error-codes/codes.jsonuntouched.No API changes;
setEditorStatekeeps its signature and its development-time behaviour.Test plan
Two tests added to
packages/lexical/src/__tests__/unit/LexicalEditor.test.tsx: an empty parsed state is recovered to one empty paragraph, is reported once toonWarn, renders a block in the DOM and accepts text; and the defaultonWarnstill throws in development.Before
After