[lexical][lexical-yjs][lexical-clipboard][lexical-html][lexical-playground] Feature: Named slots#8603
[lexical][lexical-yjs][lexical-clipboard][lexical-html][lexical-playground] Feature: Named slots#8603mayrang wants to merge 59 commits into
Conversation
mayrang
requested review from
acywatson,
etrepum,
fantactuka,
ivailop7,
potatowagon and
zurfyx
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
mayrang
changed the title
Review: $decorate hook for ElementNode + Named RootsAssessment: Impressive work, needs maintainer sign-off on API surface What I verified:
Observations:
Why this needs maintainer sign-off: This introduces new public API surface ( Reviewed by Navi (automated review assistant for @potatowagon) |
|
I haven't done a close look at this yet but I think we should consider two things:
The current DecoratorNode decorator method model is not ideal (decorate method returns some unknown value that the editor is supposed to know how to handle) because it doesn't support mixing frameworks. Being able to configure it on a per-node basis is ideal (e.g. a mutation listener would be one way). |
|
On (1): tried Walked it back across isolation rounds (manual statics one-by-one, unit tests, runtime expando trace). It isn't
Verified the children drop directly with Different concern from this PoC PR — I'm pulling it into its own PR now. Three approaches I see:
Going with the second — Will continue the On (2): a few framework-agnostic shapes are possible — two look worth pursuing: Per-node mount / update / unmount hooks. Node config exposes Two layers — a typed Two other shapes I sketched out drop pretty quickly:
Between the two viable ones: the discriminator problem hits the typed-return / adapter shape but not the lifecycle-callback shape — the typed return still hands an opaque value the adapter has to recognize (with a typed contract around it); the callbacks drop the return entirely and never have to discriminate. So the callback shape reads closer to your mutation-listener hint and the "frameworks could be mixed" goal. Trade-off is ergonomics — callbacks mean each React consumer carries |
|
I’d lean towards the lower-level lifecycle. Reducing the boilerplate should be straightforward with helper functions and/or having a way to configure the framework extension to handle that node’s lifecycle. |
|
On the clipboard fix — depends on this PR's On the lifecycle: going the lower-level callback route. Clipboard commit goes in first, lifecycle refactor next — both in this PR. |
etrepum
left a comment
etrepum
left a comment
There was a problem hiding this comment.
Overall this looks like an interesting direction, using hidden placeholders to render nodes that aren't yet mounted on the React side seems clever. I was thinking we might have to have a situation where these children don't have DOM but still need to be preserved by GC. Might be an interesting optimization someday (could be used to avoid DOM for collapsed or offscreen content for example) but I think this fits the current model better
| name: '@lexical/playground/Card', | ||
| }); | ||
|
|
||
| export default function CardPlugin(): null { |
There was a problem hiding this comment.
This should be in the extension, no need for react at all here
There was a problem hiding this comment.
Done — moved the commands into the extension's register hook and dropped CardPlugin.
|
Hadn't thought of it that way, but yeah — neat direction for lexical to go someday. |
|
Took a stab at the follow-up — added Separate fix in the next commit: Enter on a host NodeSelection and click below a trailing host both left the editor stuck (host isn't a DecoratorNode or shadow-root ElementNode, so neither the Enter handler nor |
Automated Review — $decorate hook for ElementNodeReviewer: Tater Thoughts Bobblehead (potatowagon's Navi) What I Verified✅ Architecture: Deferred-placeholder pattern is sound — children mount into hidden div until React announces real container, then relocate via DOM ops only (no editor.update loop). GC cleanup sweeps orphaned _pendingNamedRoots. ✅ Capture-guard symmetry: Create ($mountNamedRootChildren) and reconcile paths use matching guard pairs, preventing text-content cache leaks. ✅ Clipboard correctness: $childSelection(null) for NodeSelection hosts opts all descendants into export. $shouldIncludeAfterChildren promotes wrapper when every child included. ✅ React strict-mode resilience: notifyNamedRootMounted handles identity-dedup, container swap, and queueMicrotask selection re-anchor. ✅ CI: All core + canary e2e green. ✅ Test coverage: ClipboardElementDecoratorHost.test.ts covers NodeSelection, RangeSelection promotion, and round-trip. ✅ No regressions: All new hooks have identity defaults in DEFAULT_EDITOR_DOM_CONFIG. AssessmentCode quality is excellent — thorough JSDoc, sound architecture, proper cleanup. API surface still actively evolving (latest commit <1hr ago). Safe to merge once maintainers signal readiness on final API names. |
mayrang
changed the title
…target paragraph Lexical Selection convention §1.2 prefers select() / selectEnd() over selectStart() when the position is in an empty node. The target paragraph is freshly created and empty, so the two are equivalent.
…ebase onto main `pnpm run update-packages` applies the typescript-too-old guard (introduced upstream) to the new useLexicalSlot exports and sorts @lexical/code-core dependencies.
…lookup (etrepum facebook#8603) Mirrors LexicalNode.getIndexWithinParent() for slot children — looks up the slot name a node occupies on its host via the host's slot map (lazy iterate, O(slot count)).
…s ElementNode-as-host with body children + includeChildrenWhenSelected (etrepum facebook#8603) CardNode goes back to extending ElementNode with the body demoted from a named slot to regular children — title stays as the only named slot. This demonstrates the dual capability flagged in facebook#8603: an ElementNode hosting a named slot (title, one block) alongside regular children (body, N blocks). The chrome atomic UX (chrome click -> whole-Card NodeSelection, arrow boundary promotion) is kept, mirroring the earlier ElementNode iteration. Adds ElementNode#includeChildrenWhenSelected() (default false), an opt-in atomic ElementNode hosts can flip so that their children come along in clipboard / HTML output when the host is itself in a NodeSelection. Without it the Card chrome-click NodeSelection would drop body paragraphs on copy because no descendant is selected. Complements extractWithChild — that carries parent in when a child is selected; this runs in the opposite direction.
…epum facebook#8603) Following the CardNode refactor (body = regular children, not a named slot), the e2e specs lose the body-slot no-op cases (those operations now follow the normal ElementNode path), retarget the in-body click selectors to the card's direct child paragraph, flip the HTML round-trip assertion from `data-lexical-slot="body"` toContain to not.toContain, and add a cardBodyText helper that reads the card's direct child paragraphs for the collab convergence check.
…n (etrepum facebook#8603) CardExtension registers KEY_TAB_COMMAND with a small PoC handler that walks the caret's ancestors via $getSlotNameWithinHost to determine whether the caret sits in the title slot or in the body children, and moves focus accordingly: Tab from title -> body first paragraph; Shift+Tab from body -> title slot. This is the first real consumer of the helper, validating the use case (relative-order establishing for event-bubbling handlers) etrepum flagged when introducing it.
…ocus indicator (facebook#8603) The Tab handler now fires only at the slot boundary (title last paragraph end / body first paragraph start), so mid-slot Tab / Shift+Tab fall through to the rich-text indent default. Adds a `data-current-slot` attribute mirror on the active Card that CSS uses to render a caret-slot focus hint — a second use case for $getSlotNameWithinHost, mirroring the NodeSelectionDataSelectedExtension pattern but driven by caret position rather than node selection.
…tor + shadow-root semantics (facebook#8603) Reworks the playground Card chrome to read as an actual card: white background, light gray border, layered elevation shadow, rounded corners; the title slot reads as a header section with an uppercase label, a heading-weight paragraph and a thin separator; the body children sit directly inside the card with margin spacing, no extra panel chrome. Adds a caret-slot focus indicator (`data-current-slot` attribute mirrored on the active card) — only color / box-shadow mutate, no `content` / `display`, so the per-update write doesn't reflow the layout on the same frame as a forward-delete keystroke (an earlier `::before` did, dropping the first delete in Firefox / WebKit). Orange ring reads as "cursor is inside this region" and visibly differs from the blue `data-selected` ring used for the whole-card NodeSelection. Sets `Card.isShadowRoot() = true` so block-level inserts (INSERT_TABLE_COMMAND, markdown paragraph->heading transform) driven by `$insertNodeToNearestRoot` land inside the body channel instead of splitting the card.
… + selection panel (facebook#8603) `visitTree` now walks slots-first before the regular children list and passes the slot name back to the visitor so the tree-view output annotates each slot value with `[slot: <name>]`. Shadow-root ElementNodes (editable slot values) read as `[shadow-root]`; non-inline DecoratorNodes that sit in a slot (atomic decorator slot values) read as `[atomic decorator]`. The selection panel adds a slot context — when the anchor sits inside a named slot, the `range` line shows `(in slot "<name>" of <host-type> { key: ... })`.
) - LexicalSlot.test.ts: replace `as ParagraphNode` casts with `assert($isParagraphNode(host))` in three places (playbook §3.7 — `assert` over `as` cast). - LexicalSlot.ts: `$isSlotHost` / `$isSlotChild` JSDoc — clarify these are shape predicates (the type-guard side), while `$setSlot` enforces the value-level invariant (shadow-root ElementNode or non-inline DecoratorNode). - LexicalElementNode.ts: `includeChildrenWhenSelected()` JSDoc — note the clipboard caller recurses with a null outer selection, so implementers must serialize children correctly outside the original selection scope.
…lot latch + dedup $destroyNode mutation (facebook#8603) Three fixes: - `$selectAll(selection)` threw when the anchor sat on a slot value's own element point. The slot value's `getTopLevelElement()` stops at itself (slot boundary) and its `__parent` is null (the up-link is `__slotHost`), so `getParentOrThrow()` threw. Detect `topParent.getParent() === null` and scope SELECT_ALL to the slot value's own contents instead, matching the shadow-root semantics the slot boundary advertises. - `setEditorState` swapped in a parsed state without walking it, so an editor that received slot-bearing state from elsewhere (SSR + hydration, cross-editor transfer) kept `_slotsUsed = false` and silently skipped every selection clamp. Walk the new state's node map once and latch on any host with non-empty `__slots`. - `$destroyNode` had two consecutive `if (node !== undefined)` blocks with no logic between them; merged into one. Reproduce tests added at the end of LexicalSlot.test.ts pin both runtime hypotheses (both failed before fix, both pass after).
…ard, dedupe slot helpers (facebook#8603) - SyncV2: walk dropped/replaced slot subtree to delete nested binding.mapping entries (top-level delete left nested children dangling) - SyncEditorStates: gate V2 YMap dispatch on parentSub === 'slots' so other YMap attributes (e.g. __state nested maps) fall through - Utils: extract $syncSlotsFromYjsShared / $syncSlotsFromLexicalShared / $destroySlotsShared, removing ~100 lines of CollabElementNode/CollabDecoratorNode duplication
…-trip for named-slot hosts (facebook#8603) - lexical-html: forward null selection into children when host opts in via includeChildrenWhenSelected (mirrors the clipboard JSON path so a Card promoted to a NodeSelection serializes its body in HTML too) - CardNode.exportDOM: drop manual body emit — outer $appendNodesToHTML loop already recurses through getChildren() when no $getChildNodes override is supplied; manual emit caused body to land twice in the HTML - $rewrapOrphanedSlotWrappers: open a Card group per title wrapper and absorb following non-slot siblings (browser-stripped paste leaves body paragraphs at fragment root, and two title wrappers in a row must become two distinct Cards instead of one Card with the first title silently detached); cross-frame instanceof HTMLElement replaced with isHTMLElement - lexical-utils: \$dfsWithSlots / \$reverseDfsWithSlots return DFSNode[] per X[] convention
… polish (facebook#8603) - CardExtension: bail in $resolveCardChromeTarget when the click target sits inside a `[data-lexical-slot]` wrapper — the reconciler scaffold wrapper is keyless, so $getNearestNodeFromDOMNode walked past it to the Card and a click on the visible "TITLE" pseudo-label or surrounding padding silently promoted to a whole-Card NodeSelection - CardExtension: drop unreachable else-if in data-current-slot listener (activeCardKey and activeSlot are always set together inside the read block) - CardExtension: continue the $findCardSlotContext walk on a non-title slot hit instead of returning null, so future Card subclasses with extra slots still resolve the enclosing Card - FigureExtension: scope isWithinSlotEditor / $resolveFigureChromeTarget to `.editor-equation textarea/input` so an unrelated textarea inside the editor doesn't silently swallow arrows or block Figure chrome clicks - EquationComponent: refresh the slotted-host selection rationale (core selectNext now defers to the slot host, so the old "would throw" framing is stale)
…+ drop dead branch (facebook#8603) - \$slotValueMetaSuffix: bail when slotName is undefined so the [shadow-root] / [atomic decorator] marker only annotates slot values; shadow-root ElementNodes outside this feature's scope (TableCell, LayoutItem, PageContent, Collapsible) no longer pick it up - \$visitTree: drop unreachable `if (slotNode === null)` guard inside the slot iteration loop (the slotEntries array is already filtered) - \$visitTree: tighten the new tuple / indent / visitor signatures to X[] per playbook convention
… nav / SELECT_ALL scope (facebook#8603) - title slot wrapper click does NOT promote to whole-Card NodeSelection (real-browser counterpart of the unit reproduce; covers the ::before label region the unit test can't reach) - Tab from title slot moves caret into body / Shift+Tab returns it (the slot-aware key handler is the first real consumer of \$getSlotNameWithinHost, so a future refactor could regress without affecting other surface) - Cmd+A inside title slot followed by typing replaces only the title text (D5 slot-scope decision; the body must stay intact)
… attribution) (facebook#8603) - Replace the Figure (Equation slot) demo with PullQuoteNode, a DecoratorNode-as-host with two editable shadow-root slots (\`quote\` + \`attribution\`). The atomic-decorator-host pattern is preserved but the demo is now a real pull-quote UX (blog / magazine standard) and exercises the multi-editable-slot shape — every host extension (RichText, Format, Link, ...) applies inside both slots. - Remove the Figure surface: nodes/FigureNode/, plugins/FigureExtension/, unit + e2e tests, theme CSS block, picker entry. EquationNode is retained as a root-level atomic decorator demo (unchanged). - PullQuote audit fixes: PullQuoteImportRule clears seed before walking imported wrappers so a missing slot doesn't fabricate the default text; SlotCollabConvergence DecoratorNode-host case migrated to PullQuote; onChromeMouseDown shifts DOM focus off slots (mirrors Card); orphan \`attribution\` wrapper without preceding \`quote\` is dropped instead of leaking into an open Card body; CLICK_COMMAND priority aligned with Card (BEFORE_EDITOR); exportDOM iterates \`\$getSlotNames\`; attribution contrast bumped to meet WCAG AA.
…dit polish post Figure → PullQuote (facebook#8603) - EquationComponent: remove dead \$getSlotHost guards + "Figure" comments left over from the dropped Figure host (EquationNode is no longer a slot value in any playground node) - EquationNode: drop static importDOM + \$convertEquationElement — EquationImportRule in EquationsExtension covers the same data-lexical-equation match through DOMImportExtension - SerializedLexicalNode: add slots? field (@experimental) so TypeScript consumers of the JSON shape see the slot field that the export path already emits - deleteLine: narrow the slot fall-through to decorator-host slots only — element-host slots keep document order and don't need the native-range workaround - \$selectAll: guard the slot-value top-level branch with \$isElementNode so a non-inline DecoratorNode slot value doesn't crash on getChildrenSize - SyncEditorStates slot routing: replace silent null skips with invariant(hostNode !== null, ...) - ComponentPicker: PullQuote uses icon quote instead of caret-right - Add regression-pinning test for the _slotsUsed one-way latch - Cascade: LexicalNodeState.test Equal<> shape assertions include the new slots? field
…ing (facebook#8603) Manual test surfaced an edge case where the caret is at the root's element-level (typically right after deleting every top-level child) and Cmd+A then crashed with "getTopLevelElementOrThrow: root nodes are not top level elements". The slot-aware branch added by this PR calls anchorNode.getTopLevelElementOrThrow() unconditionally, but RootNode is designed to always throw from that method. The fix narrows the new slot branch to ignore the RootNode case and falls through to the regular "select all root children" path, matching the legacy $selectAll() (no-arg) behavior.
|
A few things I changed beyond what's in the body, flagging them here in case any direction is off. Figure (Equation atomic slot) → PullQuote. The original Figure demo wrapped an Equation inside a single atomic decorator slot, which from a playground user's point of view looked identical to a plain root-level decorator — the slot mechanism wasn't visible. PullQuote (one DecoratorNode host with two editable shadow-root slots, Card host shape — body as children. Earlier the Card was a two-slot host (title + body, both shadow-root slots). The body slot held an arbitrary number of paragraphs through one slot wrapper, which made the wrapper carry more than it needed to — slots are most expressive when each one is one block. With body as the host's ordinary linked-list children, the slot mechanism handles the title only and the body uses the existing ElementNode child channel. To keep the atomic UX (chrome click / arrow keys / Backspace at the host level), Card opts into a new Tab handler is narrow on purpose. Manual + e2e coverage. Added |
…rts (facebook#8603) The rebase onto origin/main resolved a conflict with facebook#8662 (Register DOMImportExtension rules implicitly via node extensions) by keeping the PlaygroundRichTextImportExtension addition, but the import block at the top of the file lost the five rich-text dependencies it references (RichTextImportExtension, ListImportExtension, TableImportExtension, CodeImportExtension, HorizontalRuleImportExtension). tsc passes but the dependencies array referenced undefined identifiers at runtime.
…facebook#8603) Resolve conflicts with the Array<T> -> T[] syntax chore (facebook#8675). In all five conflicted files (LexicalElementNode.ts, LexicalGC.ts, LexicalSelection.ts, LexicalUpdates.ts, generateContent.ts) the resolution keeps this branch's semantic changes expressed with main's T[] syntax. Also convert the Array<T>/ReadonlyArray<T> occurrences this branch introduced in LexicalUtils.js.flow and SlotSyncV2.test.ts to match the convention now enforced by @typescript-eslint/array-type. https://claude.ai/code/session_018w72jS7wzJbr14F41WTCUk
…][lexical-react][lexical-extension][lexical-devtools-core][lexical-playground] Bug Fix: Named-slots audit fixes: collab first-set sync, in-slot copy/cut, export gating, slot cycle guard (facebook#8603) Fixes every issue from the named-slots audit of this PR, in one cherry-pickable commit. Critical: - yjs V1: route a changed `slots` key in YTextEvent/YXmlEvent to syncSlotsFromYjs. The first $setSlot on a synced host (and its undo) integrates the slots Y.Map in the same transaction as its content, so no YMapEvent fires and the slot never reached peers; the follow-up peer edit then crashed on an undefined collab node or deleted the originator's slot from the shared doc. - yjs V2: make $equalYTypePNode slots-aware (and exclude the `slots` attribute from the plain attribute comparison) so the dirty scan recurses instead of repointing the mapping — the originator's first $setSlot is now actually written to yjs. - clipboard/html: walk the RangeSelection's slot frame (new core $getSlotFrame) in $generateJSONFromSelectedNodes and $generateDOMFromNodes. A selection wholly inside a slot never contains its host, so both exporters returned empty payloads and cut inside a slot was silent data loss. - clipboard/html: gate the includeChildrenWhenSelected child promotion on NodeSelection. A partial RangeSelection over a host exported the full unsliced child text plus wholly unselected siblings on both channels. Major: - selection: insertNodes with the caret on an empty slot value seeds a paragraph and redirects instead of throwing "to have a block ancestor" (reachable via Ctrl+A in an empty slot, then paste). - core: reverse cycle guard $errorOnSlotCycleChild in ElementNode.splice and insertBefore/insertAfter/replace. Closing a cycle through the children channel (slotValue.append(host)) previously hung the commit inside isAttached's up-walk. - yjs V1: sync root-host slots (the root branch never called syncSlotsFromLexical, so $setSlot($getRoot(), ...) silently never synced). - yjs V1/V2: validate-and-skip hostile or invalid remote slot entries (reserved names, non-shared-type values, missing __type, invalid slot value shapes, duplicate shared types under two names) instead of throwing inside the observer update; aliased entries are first-name- wins. - yjs: reserve the `slots` attribute key in property sync in both directions for both versions; restored V1 hosts no longer carry a junk `slots` Y.Map property, and a user property named `slots` can no longer clobber the channel. - yjs V1: sweep the transaction's deleted structs after remote events (mirroring V2) so a remote host deletion no longer leaks its slot values' collabNodeMap entries. - playground: drop CardExtension's ClickAfterLastBlockExtension override. The last-wins config merge silently clobbered the app's $isCodeNode predicate, and CardNode.isShadowRoot() already satisfies $defaultShouldInsertAfter. Minor: - clipboard: the excluded-slot-value guard also verifies the exported node type, catching the 1-child excludeFromCopy case that silently exported the child as the slot value; declare `slots` on BaseSerializedNode. - html: $appendNodeToHTML resolves the session DOM render config so disabledForSession overrides apply inside slot subtrees. - core: $setSlot re-setting the same node under the same name is a no-op instead of tripping the already-slotted invariant. - utils: $dfsWithSlots/$reverseDfsWithSlots stop before the endNode's slot subtrees (inclusive stop), document the in-slot endNode limitation, and carry @experimental tags. - react: useLexicalSlot detaches the previously appended container on nodeKey/slotName change; useCharacterLimit advances the wrap budget past slotted decorator leaves so the overflow boundary lands exactly. - extension: NodeSelectionDataSelectedExtension mirrors a NodeSelection already committed at registration, clears data-selected attributes on teardown, and raises an invariant for configured-but-unregistered classes. - yjs V2: keep the (empty) slots Y.Map on remove-last to narrow the concurrent-creation window; slot membership reads are snapshot-aware (typeMapGetAllSnapshot) so historical renders show historical slots; guard the remaining _collabNode dereferences. - devtools-core: recurse into DecoratorNode children so a decorator host's slots are visible when reached through the children channel. - playground: import rules no longer fabricate seeded "Title" content or drop a PullQuote's non-slot children (they land in the quote slot); any orphan slot wrapper closes the open rewrap run; chrome handlers are gated on editor.isEditable(); nested hosts' chrome is clickable (slot-wrapper bail only applies to the host's own slots); Tab from the title seeds an empty body paragraph when none exists; e2e slash-menu helpers use waitForSelector instead of sleep(300); CardSlotFix.spec.mjs renamed to CardSlot.spec.mjs; stale comments corrected. API modernization (new code must use the current protocols): - All node classes introduced by this commit's tests use the $config() protocol instead of legacy static getType/clone/importJSON (CardLikeNode, PlainShadowRootNode, ExcludedShadowRootNode, TestUpdateDOMTrueHostNode — the latter carrying its instance state via afterCloneFrom). The PR's other new classes (CardNode, PullQuoteNode, SlotContainerNode and the newer test nodes) already used $config(). - $getSlot drops its unchecked type parameter (the pattern deprecated by facebook#8661) — as a brand-new @experimental API it returns LexicalNode | null and callers narrow with type guards; flow declaration updated and the explicit generic call sites converted. - Every deprecated traversal type parameter in the PR's added code is scrubbed (62 call sites across LexicalSlot / LexicalSlotSelection / LexicalSlotDfs / SlotClipboardExport tests), using the same $assertNodeType(node, $isX) narrowing pattern facebook#8667 applied to the rest of the test suite; $isTestShadowRootNode / $isTestUpdateDOMTrueHostNode guards added to the shared test utils. Tests: 23 new regression tests across lexical, lexical-clipboard, lexical-yjs (including real two-doc relay observer tests for first-set, undo, hostile data, root slots, and the deletion-leak sweep), lexical-utils, lexical-react, lexical-extension, lexical-devtools-core, and the playground. Full unit suite: 169 files, 3965 passed. tsc, flow, prettier, and eslint clean. Intentionally not addressed here (need product/design decisions rather than fixes): the SELECT_ALL shadow-root scoping behavior change (kept, already e2e-pinned; recommend a separate PR + breaking-change note), per-client slot Map ordering under concurrent distinct-name adds (inherent to the Y.Map channel design), and a mixed-version rollout note for pre-slots V1 clients. https://claude.ai/code/session_0125UE7Cv1mUaM82UH8N1nJX
…n-text][lexical-extension][lexical-playground] Feature: Canonical slot order via $config + slot-frame-scoped SELECT_ALL (facebook#8603) Two design changes agreed in review, plus their SelectBlockExtension integration: Canonical slot order (convergent, declaration-driven): - StaticNodeConfigValue gains `slots?: readonly string[]` — a host class declares its slot order in $config. getDeclaredSlots(klass) resolves the nearest declaration in the prototype chain (subclasses may redeclare); declarations are validated once (no duplicates, no reserved names). - $setSlot canonicalizes the host's slot map on every write: declared names first in declaration order, undeclared names after in code-unit order. Order is derived, never stored — every ingestion path (local API, JSON import, clipboard, V1/V2 collab sync) funnels through $setSlot, so documents re-canonicalize on load and concurrent collaborative additions converge to the same order on every client. The declaration is not a schema: undeclared names remain valid and sort into the deterministic tail, so adding, reordering, or retiring declared names over time is non-destructive. - Declared hosts opt into eager slots Y.Map creation in both collab versions (V1 $seedHostSlots; V2 $createSlotsYType on the creation path AND $updateSlotsYType on the update path), so each name's first set merges per-entry under concurrency instead of racing on attribute-level LWW. The V2 creation-path gate was missing in the first cut and is pinned by a regression test that demonstrated the race (concurrent first-sets of different names previously converged to a single surviving slot). - CardNode declares slots: ['title']; PullQuoteNode declares slots: ['quote', 'attribution'] (code-unit order would have inverted it). The "remove + re-add moves to the tail" insertion-order test is replaced by canonical-contract tests (late adds land in canonical position in model and DOM; remove + re-add returns to it). Slot-frame-scoped SELECT_ALL (conservative default): - The rich-text/plain-text SELECT_ALL handlers now scope to the selection only when the caret is inside a named-slot frame ($getSlotFrame). Every other context — including TableCell shadow roots — keeps the legacy whole-document behavior, so the earlier table-cell behavior change is reverted along with its e2e adaptations (Tables.spec.mjs and Indentation.spec.mjs are restored to main, reviving the original "Can delete all with range selection anchored in table" guarantee). Scoped/progressive select-all elsewhere is the opt-in SelectBlockExtension's job. SelectBlockExtension integration: - $isBlockFullySelected is slot-frame aware: a range and a block in different slot frames can never be a full selection, and comparing them previously walked a caret comparison across the parentless slot boundary (no common ancestor). With the guard, the extension's press-again-to-expand escalates from an in-slot block to the whole document without throwing. - New compose suite (SelectBlockSlots.test.ts): first press selects the in-slot block, second press expands to the document, the frame guard returns false instead of throwing, and the disabled-extension path falls back to the slot-scoped rich-text default. - The playground now enables SelectBlockExtension by default (appSettings selectBlock: true). The e2e harness already pins every setting into URL params on initialize(), so the existing suite (28 spec files / 87 selectAll call sites) keeps legacy semantics and SelectBlock.spec.mjs keeps its explicit opt-in; the Settings UI default inversion is handled dynamically. Tests: canonical-order permutations (declared, mixed, undeclared, subclass redeclaration, DOM container order, exportJSON key order, declaration validation), V1 offline two-client convergence (concurrent first-sets of different names both survive on a declared host and converge to declared order; declared + undeclared concurrent adds converge), V2 eager-map creation and order-canonicalizing restore, PullQuote reversed-set canonicalization, and the SelectBlockExtension compose suite. Full unit suite: 177 files, 4051 passed. tsc, flow, prettier, eslint clean. https://claude.ai/code/session_0125UE7Cv1mUaM82UH8N1nJX
|
I went through the open issues to see which ones Named Slots could plausibly land a follow-up fix for. Pending actually building them, I'd like to try:
There might be more once I actually start building them. I'll start picking them up after this PR is merged. |
…playground] Refactor: $create in slot tests + copy-on-write slot maps (facebook#8603) Two review follow-ups: $create in tests: - Every direct `new XNode()` construction in the slot test suites (27 sites across the clipboard, core, yjs V1/V2, extension-compose, and PullQuote tests) goes through $create(XNode) so the tests model replacement-compatible construction; the local $createXNode factories delegate to $create. Copy-on-write slot maps (mirrors NodeState's owner scheme): - afterCloneFrom on ElementNode/DecoratorNode shares the __slots Map reference across versions instead of eagerly copying it, so a host cloned for any non-slot change (every keystroke in a Card body re-clones the host via splice) no longer allocates a Map per version. - The LexicalSlot mutators clone exactly once per writable version on its first write, tracked through a SLOT_MAP_OWNER symbol property stamped on each map at construction (measured ~2.6x faster than a WeakMap ledger on the owner paths and the smallest of the options; Map read ops are unaffected by the expando). Copy-on-write means the owner is only ever written to a freshly constructed map — a shared or committed map is replaced, never mutated — and a plain `new Map(m)` clone is born unowned because expandos don't copy. $setSlot / $removeSlot / canonicalization all route through $getWritableSlots; readers are untouched. - $cloneWithProperties' dev invariant now passes through its identity fast path for shared maps. - One legacy test predating $removeSlot mutated the writable's __slots directly, which under sharing corrupts the committed previous version instead of diffing against it; it now uses $removeSlot. Direct map mutation was never a supported write path (__slots is @internal, the mutators are the sanctioned writers — same contract as NodeState). - New tests pin the COW contract: a non-slot host clone shares the map reference; a slot write clones once per version and reuses the owned clone for further writes in the same update; previously committed states keep their own map and contents. Full unit suite: 177 files, 4053 passed. tsc, flow, prettier, eslint clean. https://claude.ai/code/session_0125UE7Cv1mUaM82UH8N1nJX
…n Linux Firefox (facebook#8603) The Linux Firefox selectAll helper emulates a native whole-document selection (setBaseAndExtent from the document's deep-first position, facebook#4665) instead of pressing the shortcut, so SELECT_ALL_COMMAND never fires there: the slot-scoped behavior these three tests assert cannot be produced, and the emulated selection is anchored at the document start rather than at the caret inside the slot — typing after it replaces the whole document (failing with cardCount 0 / attribution null on firefox in every editor mode, plain and collab alike). Skip them on firefox + Linux with the same guard SelectBlock.spec.mjs uses for the same reason (it depends on the command path identically). Chromium and WebKit continue to exercise all three through the real shortcut. Verified locally: firefox plain and rich-text-with-collab both run 15 passed / 3 skipped across CardSlot.spec.mjs + PullQuoteSlot.spec.mjs; chromium runs all 18. https://claude.ai/code/session_0125UE7Cv1mUaM82UH8N1nJX
…Linux Firefox (facebook#8603) The Linux Firefox branch of the selectAll e2e helper emulated a native whole-document selection with setBaseAndExtent instead of pressing the shortcut — a facebook#4665 workaround that facebook#8532 made obsolete. The emulation bypassed SELECT_ALL_COMMAND entirely and anchored at the document's deep-first position rather than the caret, so any command-scoped behavior was untestable on that platform: the named-slot SELECT_ALL specs failed (typing after select-all replaced the whole document, destroying the Card / the PullQuote attribution) and SelectBlock.spec had to skip firefox outright. Delete the fork so every browser presses the real shortcut, and remove SelectBlock.spec's firefox+linux skip along with it. Verified on Linux Firefox locally: all 26 selectAll-consuming e2e spec files pass (443 passed, 0 failed; remaining skips are pre-existing unrelated platform guards), including the three previously-failing slot specs and the full, no-longer-skipped SelectBlock suite. https://claude.ai/code/session_0125UE7Cv1mUaM82UH8N1nJX
…o feat/5930-element-decorate
4 participants
Description
Lexical's nested editors put each region in its own editorState, so moving nodes between regions and keeping history and collab in sync all need serialization and extra
editor.updatepasses (#5930). Named slots keep those regions in the host's editorState as a second child channel rendered into the host's own DOM, so editing a slot is just editing the one tree.Named slots let a single host ElementNode own several isolated editable regions addressed by name (a Card's
titleandbody, a PullQuote'squoteandattribution) instead of sharing one child linked list. issue #5930 asks for exactly this: regions that each take their own caret and formatting, never merge across the boundary, and don't letCmd+Aspill into the rest of the document. The existing node kinds can't express it — plain ElementNode children are one undivided linked list, so backspace at a region start merges it into the previous region, and a DecoratorNode is atomic, so Lexical can't own selection / collab / serialization inside it.The model: a host keeps a second child channel,
__slots: Map<slotName, NodeKey>, separate from its__first/__lastlinked list. A slot value has__slotHostset and__parent === null, with exactly one of the two non-null, sogetParent()stops at the slot boundary and you climb out only throughgetSlotHost(). Isolation is structural rather than a convention: an accidental boundary crossing surfaces as a thrown invariant, not silent corruption. Slots render slots-first, each wrapped in<div data-lexical-slot="<name>">.This is
@experimental. Two playground demos cover both slot shapes: Card (two editable text slots, each aSlotContainerNodeshadow-root wrapper around a paragraph) and PullQuote (one DecoratorNode host with two editable shadow-root slots:quoteandattribution).Design notes
Map<string, NodeKey>, not a plain object. Slot names are arbitrary strings, and a plain object lets"__proto__"or"constructor"produce a falsehas().Maphas no prototype keys.__slotHostpointer; recover the name from the host's Map. One source of truth removes a whole sync-bug class, and slot counts are tiny so the reverse lookup is free. Worth revisiting at hundreds of slots, not for cards.Cmd+Ahas no single block to scope to and spills document-wide, and a bare TextNode has nothing to anchor against on the stable CollabElementNode path. This mirrorsRootNoderejecting non-block direct children. The rule lives at the singlesetSlotchokepoint, so every create / restore path conforms.isAttachedis the GC linchpin. It now follows__slotHostwhen__parentis null, so a live slot is no longer seen as detached and reaped; fixing this one function carries slot-safety through GC. A second guard throws if a filled slot is dropped on export, as a data-loss tripwire.getTextContent,getAllTextNodes): "what is in this subtree" has to count slot content for search, copy, and accessibility. Navigation excludes them (getFirstDescendantand friends feedselectStart/selectEnd, so including slots would walk the caret into a slot and break isolation).getChildren()stays linked-list-only for backwards compatibility; slots get a newgetSlots().isEmpty()is slot-aware (childrenSize === 0 && slotNames.length === 0).$removeNodecascade-prunes empty parents, so a slots-only host that lost its last linked-list child would otherwise be reaped and orphan its slots.$internalResolveSelectionPoints), the API ($setSelection), and in-place point mutation. They share one helper with the direction test injected per caller. The clamp is anchor-frame, so a cross-boundaryshift+arrow(whichmodifyresolves as a RangeSelection bound to the anchor's shadow root) and a mouse drag land on the same result instead of splitting into RangeSelection for the keyboard and NodeSelection for the mouse.slotsY.Map.__slotsand__slotHostjoin the synced-property exclusion sets, and slots serialize through a per-slot-diffedY.Mapon the host attribute channel on both the V1 (_xmlText) and V2 (XmlElement) paths.slotskey. NodeKeys aren't serialized, so the name is the identity, and a dedicated key is the natural place for it.includeChildrenWhenSelected. A new opt-in flag on ElementNode keeps a host's chrome (arrow keys, mouse click, Backspace on a NodeSelection) at the host level even though its slots are walkable subtrees. Without it, an arrow key from the surrounding paragraph would dive straight into the title slot, and Backspace on a selected Card would erase the host's first child instead of the host. The flag is per-node, so ordinary ElementNodes are unaffected.$rewrapOrphanedSlotWrapperson the HTML import path. HTML export emits each slot as a<div data-lexical-slot="<name>">, but a round-trip through an external editor can strip the host's outer wrapper while leaving the slot wrappers in place. The import preprocess walks the parsed DOM, finds those orphaned slot wrappers, and re-parents them back under a freshly created host so JSON imports stay strict while HTML imports stay best-effort.Two gaps remain. First, the caret / NodeCaret APIs throw across a slot boundary ("no common ancestor"), so the clamp resolves its direction with a model comparator instead of the caret system. Second, nested slots (a slot whose host is itself slotted) are out of scope at the runtime layer here: the model comparator reduces a slot-internal point to its host only one level deep, and neither demo nests slots, so the deeper case is left for a follow-up. The static lookup side (e.g.
$getSlotNameWithinHostreturns the immediate host's slot name when the slot value is itself a slot host) is already covered.Closes #5930
Closes #6613
Test plan
pnpm tsc --noEmit -p tsconfig.json/-p tsconfig.test.jsoncleanpnpm flow— no errorspnpm vitest run --project unit— full unit suite green (slot suites + reconciler / selection / node / html / yjs-sync regression)CardSlotFix.spec.mjs/PullQuoteSlot.spec.mjs/SlotCollabConvergence.spec.mjsgreen — 12 matrices (chromium / firefox / webkit × rich-text / plain-text / collab v1 / collab v2)<div data-lexical-slot>wrappers stable after re-renderquote/attributionslots, slot isolationCmd+Backspaceinside a slot deletes only slot-internal characters, on a NodeSelection deletes the host;Cmd+Ainside a slot stays scoped to that slot,Cmd+Aat the root selects root-level content without absorbing slot-internal textCmd+A(fixed by$selectAllRootNodeguard in the most recent commit); nested host; undo / redo across host insert / delete and slot-internal typing; collab delete during IME composition; Korean composition inside a slot — no leak across slot boundary