[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.
…lize # Conflicts: # packages/lexical-html/flow/LexicalHtml.js.flow # packages/lexical/flow/Lexical.js.flow # packages/lexical/src/LexicalReconciler.ts # packages/lexical/src/nodes/LexicalElementNode.ts
|
Conflicts resolved (also picked up #8536's |
| flat.push(childJson); | ||
| } | ||
| }); | ||
| json.children = flat; |
There was a problem hiding this comment.
This doesn't work, exportJSON is not responsible for children (always has an empty array), the children traversal happens afterwards.
There was a problem hiding this comment.
Right, missed that exportJSON runs before children push. For the grouped→flat transform — was the intent a node transform on the 'moved out' boundary you mentioned, or somewhere else (parse side, exportDOM)? Want to confirm the layer before retrying.
There was a problem hiding this comment.
Honestly not sure there is an approach here with the existing API since you're changing the children list, excludeFromCopy will let you omit it but it won't let you inject a LineBreakNode. Maybe if the LineBreakNode was still there but display: none or something it might work better without changing the JSON API.
There was a problem hiding this comment.
On the grouped CodeShiki direction — checked the verify: $getDOMSlot override is per-node single-slot, so it can't partition a CodeNode's children into per-line wrappers. The "LineBreakNode + display:none / display:contents" path you mentioned sits outside this PR's scope (DOMSlot generalize + DOMRenderExtension surface + the leaf-node / $decorateDOM / extension-migration examples).
Considering splitting the grouped mode commits (CodeLineNode wrapping + per-line wrap-mode line-numbers, separate from the $decorateDOM-based CodeGutterExtension which stays) out so #8519 keeps the parts that match its main feature, and the CSS-based line-numbers becomes a follow-up. What do you think of that direction?
There was a problem hiding this comment.
Does the $decorateDOM based code gutter solve any UX issues and/or considerably simplify the code, or is it just churn to demonstrate the feature? I think it’s probably not worth including until we have a version that’s motivated by UX progress.
There was a problem hiding this comment.
Yeah, agreed — the gutter / grouped mode commits are coming out. Will keep them on a side branch in case a UX-motivated version wants to pick from them.
Side note, not blocking this PR — I've been poking at #5930 lately. Did a small PoC of the ElementNode-as-decorator side on top of #8519's hooks; outer DOM + state path lands cleanly via $createDOM + $decorateDOM, but the React strict-mode unmount/remount cycle keeps detaching the named-root children. Feels like a more natural follow-up to #8519 than the gutter rewrite — would want to RFC the lifecycle hook side properly if there's interest.
There was a problem hiding this comment.
We might be able to implement react decorators as a $decorateDOM to facilitate that
There was a problem hiding this comment.
$decorateDOM as the React decorator entry — after this PR lands I'll scope the RFC and look at how it handles the unmount/remount detach the PoC hit.
|
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