[Breaking Change][lexical][lexical-html][lexical-selection][lexical-utils][lexical-playground] Feature: Generalize DOMSlot and add DOMRenderExtension override surface - #8519
Conversation
…Slot/ElementDOMSlot split
…via extension (visible linebreaks)
… DraggableBlockPlugin around slot-injected handle
R6 (API surface + JSDoc + edges): - Move BlockDragHandleExtension JSDoc above the actual symbol; add per-export docs to BLOCK_DRAG_WRAPPER_ATTR / BLOCK_DRAG_INNER_ATTR / BLOCK_DRAG_HANDLE_ATTR; correct the useDecorators portal-mount caveat. - Drop @internal from $getEditorDOMRenderConfig / $getElementDOMSlot / ElementNode.getDOMSlot (cross-package callers, @experimental is the right envelope). - Move BlockDragHandleExtension registration from playground App scope to PlaygroundRichTextExtension so plain-text mode does not render inert handles. - Flow: DOMSlot<T> generic fidelity. R7 (collab / lifecycle / composition): - createDOMRange (lexical-selection) routes ElementNode anchor/focus through \$getElementDOMSlot so Yjs remote cursors land on the content element, not the wrapper gutter. - \$getComputedStyleForElement reads from the slot's inner element so an extension wrapper does not shadow writing-mode / direction. - HR / PageBreak React click handlers use contains() against the slot's inner element, not the keyed DOM (the wrapper); drops the [data-lexical-block-drag-handle] guard string. - TextNode setTextContent comment strengthened: append-only is safe on vanilla TextNode, prepend/wrap requires a subclass override. R8 (browser quirks / node-type interactions / DnD math): - BlockDragHandleExtension.\$getDOMSlot dispatches to node.getDOMSlot(inner) so TableNode's withElement(table).withAfter(colgroup) is preserved (otherwise scrollable tables inserted rows into the outer div). - Restore .draggable-block-component-picker / .component-picker-search CSS lost in the DraggableBlockPlugin rewrite. - getBlockElementFromHandle uses :scope > for the inner lookup. - Handle CSS adds -webkit-touch-callout: none + user-select: none so iPad long-press does not surface the OS context lens. R9 (test coverage + migration docs): - LexicalBlockDragHandleExtension.test.tsx: HR (DecoratorNode) wrap test, TableNode scrollable-mode wrap regression test. - LexicalElementNode.test.tsx: resolveChildIndex test covering DOM-offset to lexical-index mapping with non-zero firstChildOffset, including clamp boundaries. - DraggableBlockPlugin_EXPERIMENTAL JSDoc: explicit breaking-change note for removed menuRef / menuComponent / isOnMenu props. - plugins.md: migration reference link to examples/website-notion/src/plugins/DragPlugin.tsx promoted. R10 (audit of R6-R9 routing changes): no critical or important findings.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
`triggerCommandListeners` already wraps each listener bucket in
`updateEditorSync`, so `$commitSuggestion` runs in an active editor
context inside the Tab handler. Wrapping in `editor.update({discrete:
true})` from inside that existing update deferred the splice to a
microtask, leaving `didCommit` stuck at `false` long enough for the
EDITOR-priority tab-indentation handler to insert a `<tab>` before the
suggestion landed. Direct `$commitSuggestion()` call instead — the
splice runs synchronously, `didCommit` reflects the real outcome, and
the suggestion-commit case correctly stops propagation.
Touch handler keeps its own `editor.update` wrapper since it is not
called from a command-listener pipeline.
Caught by the rich-text e2e canary on chromium (Autocomplete.spec.mjs
"format-as-the-original-text" and "Undo does not cause an exception")
where the trailing tab created an extra empty <strong> with the
TabNode theme class.
…orator / void cases
The first revision of `prettifyHTML`'s wrapper strip used a non-greedy
regex which terminated at the first `</TAG>` it saw. For an embed-block
wrap (inner is a `<div>` containing nested `<div>` content) the match
stopped at the embed's inner `</div>` instead of the inner-marker's,
so the wrapper scaffolding stayed in the snapshot. Switched to a
depth-aware walker (`findMatchingClose`) and split the strip into three
cases matching the three observed structural shapes:
- ElementNode (paragraph / heading / list etc.) — inner element carries
the real attributes; merge wrapper attrs (dir / style picked up by
the reconciler on the keyed DOM) into the inner element, drop the
wrapper. Wrapper attrs already present on the inner (e.g. an empty
`style=""` placeholder from indent toggling) are skipped to avoid
duplicate attributes.
- DecoratorNode with void inner (HR) — the `<hr>` itself is the
canonical content; merge wrapper attrs into it and emit self-closing.
- DecoratorNode with React portal (embed, PageBreak) — `useDecorators`
mounts the portal as a sibling of the empty inner-marker since the
keyed DOM is the wrapper; keep the wrapper minus the marker attr and
drop the empty inner shell.
`assertSelectionOnPageOrFrame.getPathFromNode` now skips the inner-marker
step when walking up to the contenteditable root, so DOM paths in
selection assertions match the pre-wrap depth.
Tables: `nthTableSelector` switched to Playwright's `:nth-match` since
top-level tables are no longer siblings under wrap. TextFormatting
triple-click test: changed `> p` to descendant `p` so the inner
paragraph (now wrapped) is reachable.
Also fixes a separate Autocomplete commit issue caught at the same
time: the Tab handler wrapped `$commitSuggestion` in
`editor.update({discrete: true})`, but the handler is already inside a
`triggerCommandListeners`-driven `updateEditorSync`, so the nested
update deferred to a microtask. `didCommit` came back as `false`,
propagation continued, and the EDITOR-priority TabIndentation handler
inserted a `<tab>` before the suggestion landed. Direct
`$commitSuggestion()` call instead; touch handler keeps its own
`editor.update` since it isn't called from the command pipeline.
…ctors
The wrap-all approach attempted to give DecoratorNodes (HR, PageBreak,
equation, image) drag handles by wrapping their keyed DOM with a
sibling button + an inner-marker. The latent issue documented in R6/C2
turned out to bite at scale: `useDecorators` mounts the React portal
via `editor.getElementByKey`, which returns the wrapper, so any
decorator whose `decorate()` renders visible content (EquationNode,
ImageNode, etc.) lands the portal as a sibling of the empty inner
shell with undefined visual ordering. Narrow `shouldWrap` to top-level
ElementNode only — DecoratorNodes keep their original keyed DOM and
forgo the drag handle for now. The slot abstraction can cover portal
routing in a follow-up if the wrap-all extension pattern catches on.
Drag handle textContent dropped (`⋮` was a fallback over the
background-image visual). Adding text content to the wrapper interfered
with browser triple-click selection on multi-paragraph documents:
the wrapper became a text-bearing block and the multi-paragraph
triple-click collapsed to an empty selection. The visual is purely the
CSS background-image now, with `aria-label="Drag to reorder"` for
accessibility.
`prettifyHTML`'s wrapper strip generalised for the three structural
shapes observed under wrap:
- ElementNode (paragraph etc.): merge wrapper attrs into inner element,
drop wrapper. `class` and `style` are composable — wrapper-side
tokens are appended to the inner's existing value (so e.g.
alignment-applied `style="text-align: center"` on the wrapper merges
with the inner's existing class). Other attrs (`dir` etc.) keep the
inner's value when both define them.
- Void inner (e.g. HR under the legacy wrap, kept for safety): merge
wrapper attrs into the void element, emit self-closing.
- DecoratorNode wrap (no longer applied today but kept for safety):
empty inner shell + portal-mounted sibling content. Wrapper-minus-
marker becomes the result tag.
`assertSelectionOnPageOrFrame`:
- `getPathFromNode` skips the inner-marker layer.
- `normalizeWrapperFocus` translates wrapper-as-focus positions (which
arrow-key navigation parks at as block boundaries) to the deepest
first / last text inside the inner so paths line up with the
un-wrapped baseline.
Spec-level fixes:
- `dragDraggableMenuTo` accepts a `fromBlockSelector` so callers can
scope the handle lookup to the source block under wrap.
- Direct-child selectors (`div[contenteditable="true"] > p`,
`.ContentEditable__root > .paragraph`, etc.) switched to descendant.
- `:nth-child` / `:nth-of-type` on per-block elements like
`tableScrollableWrapper`, `editor-image` switched to Playwright's
`:nth-match` since the wrap collapses sibling structure.
- `Mutations.spec.mjs` "Remove the paragraph" targets the wrapper
(the visible top-level block); inner-element manipulations descend
through `querySelector('p')`.
- `Extensions.spec.mjs` insertText test traverses via
`editor.querySelectorAll('p')[1]` instead of `firstChild.nextSibling`.
Also revives the Autocomplete commit fix (direct `$commitSuggestion`
without nested `editor.update`) and the unit test for "HR isn't wrapped"
to lock the narrowed `shouldWrap` contract.
Known remaining wrap-systemic regressions (not addressed in this
commit — needs a deeper look at lexical's selection mapping or a
wrap-design revisit):
- Browser arrow-key navigation across wrap boundaries lands selection
on the wrapper element. `execCommand("insertText")` on a wrap-
spanning selection drops the inner `<p>`.
- Various text-transform-after-keystroke tests (Hashtags, Mentions,
Navigation) are affected by cursor positioning across the wrap.
|
I don't have any time to look closer at this but claude seems to think that the latest merge commit reverted some stuff unintentionally Claude's analysis (unverified)#8548 is silently undone packages/lexical/src/LexicalReconciler.ts (post-merge, line 561): function $reconcileElementTerminatingLineBreak(...) { const slot = activeEditorDOMRenderConfig.$getDOMSlot(nextElement, dom, activeEditor); Net effect: the trailing- #8547's Flow migration is mass-reverted ElementDOMSlot ← reverted to → <+T extends HTMLElement = HTMLElement> packages/lexical-html/flow/LexicalHtml.js.flow survived correctly — main's readonly $getDOMSlot? modernization was preserved and only the return-type generalization (ElementDOMSlot → DOMSlot) was applied. TS/Flow inconsistency on DOMRenderMatch.$getDOMSlot $getDOMSlot?: ( readonly $getDOMSlot?: ( What merged correctly |
… merge dropped The merge resolution that resolved conflicts via `git checkout --ours` inadvertently reverted main changes that the same files needed. - `LexicalReconciler.ts`: `$reconcileElementTerminatingLineBreak` now reads `prevLineBreak` from `slot.element.__lexicalLastChildKind` (cache) instead of calling `isLastChildLineBreakOrDecorator(prevElement, ...)` on a detached prev-state node — restores the facebook#8548 crash fix. - `LexicalDOMSlot.ts`: `setManagedLineBreak` writes the cache field that the reconciler reads — without this the field declaration was orphan. - `Lexical.js.flow`: migrates remaining `+T` / `+property` to the new `out T` / `readonly` syntax to match facebook#8547's flow migration on main.
…getInsertionAnchor method
…acebook#8561) $updateDOMBlockCursorElement inserted the block cursor into the node's keyed DOM. For a node whose getDOMSlot wraps its content (keyed DOM !== slot.element), the managed children — and therefore the reference node the cursor must sit before — live in slot.element, so the insert threw "NotFoundError: The child can not be found in the parent" and the edit never displayed. The block cursor can also sit between two block children, which the slot's child-index mapping must account for. - $updateDOMBlockCursorElement: resolve the container via $getDOMSlot so the cursor lands in the content-bearing element. Renamed from updateDOMBlockCursorElement (internal; requires an active editor). - DOMSlot.getFirstChildAnchor: new leading-boundary counterpart to getInsertionAnchor (the node the managed range starts after). Base returns this.after; ElementDOMSlot extends it to skip a head block cursor, just as getInsertionAnchor extends this.before past the managed line break. getFirstChild uses it; only ElementNodes host a block cursor, so the base slot stays editor-free. - ElementDOMSlot.getFirstChildOffset: walk forward counting leading non-lexical nodes (this.after region + a head block cursor), stopping at the first managed child or the trailing boundary (this.before / managed line break) so empty elements don't overcount. - ElementDOMSlot.resolveChildIndex: count managed children up to the DOM offset, skipping a block cursor interleaved between two block children (its DOM slot is not a lexical child); the previous arithmetic assumed children occupied a contiguous DOM range. - markSelection: read with {editor} so the active editor is set when $rangeTargetFromPoint reads the slot. Behavior-driven regression tests (focused editor, collapsed element selection before a block decorator inside a getDOMSlot wrapper) built with buildEditorFromExtensions fail without the fix with the facebook#8561 NotFoundError. Adds ElementDOMSlot unit coverage for getFirstChild / getFirstChildOffset / resolveChildIndex with a head, non-head, and interleaved block cursor and the trailing-boundary stop; the existing slot unit tests now run inside an editor update.
…ot override $setTextContent resolves its slot via the editor-level $getDOMSlot hook ($getEditorDOMRenderConfig(editor).$getDOMSlot), so a DOMRenderExtension can reroute a TextNode's text writes through a custom slot — e.g. when it injects a contentEditable=false sibling around the text. That path had no coverage: the existing TextNode overrides only exercise $exportDOM / $decorateDOM, and the autocomplete e2e uses the default slot (the pre-slot code already preserved an appended sibling via nodeValue), so neither pins the reroute. Adds a DOMRenderExtension that parks a contentEditable=false prelude as the first child of each TextNode span and exposes the text-bearing range via slot.after. An in-place text edit must consult the override (not dom.firstChild) to update the text node after the prelude; the test fails if $setTextContent bypasses the editor hook (it would mangle the prelude or append a duplicate text node).
Convert the slot/selection regression tests to the repo's vitest
`assert(value, message?)` convention: replace `invariant(...)` calls and
`if (guard) { throw new Error(...) }` guards with `assert(...)`, which
also narrows types for the type checker.
https://claude.ai/code/session_012GJUM1UwDupZ16eEYXfFe1
Revert the lexical-selection test back to its upstream form. Swapping `expect($isX(node)).toBe(true)` for `invariant($isX(node), ...)` changed nothing meaningful here, so restore the original assertions. https://claude.ai/code/session_012GJUM1UwDupZ16eEYXfFe1
Replace `ReturnType<typeof buildEditorFromExtensions>` with the named `LexicalEditorWithDispose` type, and have the wrap-pattern tests call the shared `mountRoot` helper (now returning the root) instead of pushing to `mountedRoots` inline. https://claude.ai/code/session_012GJUM1UwDupZ16eEYXfFe1
The DOMSlot implementation is no longer tightly coupled to LexicalElementNode, so relocate the ElementDOMSlot describe blocks into a dedicated LexicalDOMSlot.test.tsx. While moving, build editors with buildEditorFromExtensions/defineExtension, mount via setRootElement, and declare the test node classes with $config instead of static getType / clone / importJSON / exportJSON boilerplate. https://claude.ai/code/session_012GJUM1UwDupZ16eEYXfFe1
etrepum
left a comment
There was a problem hiding this comment.
I think this is in good shape but will do some more auditing before merge since this is such a big change
| console.timeEnd('query'); | ||
| return response; | ||
| }, []); | ||
| function query(searchText: string): SearchPromise { |
There was a problem hiding this comment.
If you do refactor this to demonstrate better IME support I think we should construct the server in the extension's build, and probably use a more standard AbortController/AbortSignal when communicating with the server function
There was a problem hiding this comment.
Got a composition-idle IME branch on top of an older AutocompletePlugin base (pluggable dictionaries + Korean pilot + idle ghost). Re-porting that on top of the new Extension shape was already the post-merge plan; the build-time server construction and AbortController/AbortSignal both fit cleanly into that re-port. Will follow up after this lands.
Breaking changes
TextNode's DOM update of its Text content routes through$getDOMSlotof the DOMRenderConfig. The default slot path is equivalent to the previousdom.textContent =behavior; TextNode subclasses with a customgetDOMSlotoverride now scope text updates to the actual text-bearing child instead of wiping the wrapper.$generateHtmlFromNodesrequires an editor context to find the DOMRenderConfig, so if you are using it witheditorState.read(() => $generateHtmlFromNodes(…))you will need to transform that call toeditorState.read(() => $generateHtmlFromNodes(…), {editor})so that an editor is in the context.Description
getDOMSlotonly exists onElementNode. The slot abstraction (separating the keyed DOM from the content-bearing element + lexical-managed children range) is useful beyond children placement — leaf nodes also have cases where a wrapper sits between the keyed DOM and the canonical content node (a styled<br>from #8201, acontentEditable=falsesibling inside a TextNode's span). And there's been no extension-level hook to inject that wrapping without subclassing the node.This PR lifts
getDOMSlottoLexicalNode, splitsDOMSlotinto a base class (any node) andElementDOMSlot(children management on top), and adds an editor-level$getDOMSlotoverride surface throughDOMRenderExtension. The reconciler, mutation observer, selection plumbing, andTextNode.setTextContentall route through the slot'selement/before/after/getFirstChild()rather than touching keyed DOM directly, so extensions can wrap a node's DOM without subclassing.Two worked examples sit on top, one per leaf-node axis (leaf wrap, TextNode sibling decoration):
VisibleLineBreakExtension(@lexical/playground) — extension-driven leaf wrap. EachLineBreakNode's<br>ends up inside a<span>carrying a↵marker, with the inner<br>exposed through the slot so selection / caret logic still targets the canonical content element.@lexical/playground) — extension-driven TextNode sibling decoration.AutocompleteNodeis deleted entirely; the ghost is now acontentEditable=falsesibling inside the active TextNode's span, markedsetDOMUnmanagedso the mutation observer leaves it alone.What changed
LexicalDOMSlot.tsis the new home of the base class withelement/before/after/insertChild/removeChild/replaceChild/withBefore/withAfter/withElement/getFirstChild/resolveLeafPosition.ElementDOMSlotextends it and addsgetManagedLineBreak/removeManagedLineBreak/insertManagedLineBreak/getFirstChildOffset/resolveChildIndex.ElementDOMSlot.resolveChildIndexreturnsclamped - firstChildOffsetso a slot with non-zerogetFirstChildOffset()maps DOM offsets back into lexical-child index space.DOMSlot.resolveLeafPositiontranslates DOM caret offsets inside a wrap to a "before vs after" decision relative to the canonical content element.getDOMSlothook onLexicalNode. Default returnsnew DOMSlot(element).ElementNodekeeps its override (returnsElementDOMSlot).TextNode.setTextContentnow routes throughnode.getDOMSlot(dom)andslot.getFirstChild()/slot.insertChild()so a TextNode subclass with a slot override can scope text updates to the text-bearing child instead of wiping all siblings viadom.textContent =.DOMRenderExtension$getDOMSlotoverride. Editor-level configuration in@lexical/html'sDOMRenderExtensionchain. Selection plumbing ($createDOMRange,$getComputedStyleForElementin@lexical/selection;markSelectionin@lexical/utils) routes through this hook.LexicalSelection.ts's leaf branch now defers toslot.resolveLeafPositionwhenslot.element !== leafDOM(wrap pattern), and preserves the historical decorator-only "before" rule for bare leaves.slot.withBefore(beforeDOM ?? slot.before)fallback. Preserves the slot's ownbeforeboundary when a reconcile step passesnull.setDOMUnmanaged/isDOMUnmanaged.@experimental(was@internal).Backwards compatibility
ElementDOMSlot's pre-existing API surface is frozen — signatures unchanged, behavior under the defaultfirstChildOffset === 0path is identical.LexicalNode.getDOMSlotdefault returnsnew DOMSlot(element); call sites that read element-level slot config keep returningElementDOMSlot.setDOMUnmanaged/isDOMUnmanagedwere@internalexports already used byTableNodeandCodeNode; the only change is the documentation tag.Closes #8201
Closes #8561
Test plan