[lexical-selection] Bug Fix: Handle bare slot values in $setBlocksType - #8901
Merged
Conversation
mayrang
requested review from
acywatson,
etrepum,
fantactuka,
ivailop7,
potatowagon and
zurfyx
as code owners
July 31, 2026 15:59
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
etrepum
reviewed
Jul 31, 2026
etrepum
left a comment
etrepum
left a comment
Collaborator
There was a problem hiding this comment.
I think the expected behavior here is that $setBlocksType should just not do anything if you call it on a named slot (no parent)
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 1, 2026
…alues in block converters and diagnose replace() on a slot value ## Description Stacked on facebook#8901 (fix for facebook#8894), applying the review feedback: instead of $setBlocksType reassigning the slot itself via $setSlot, a named-slot value should simply not be eligible for generic block conversion — the slot assignment is managed by the node or extension that owns the slot. This reverts the in-converter $setSlot branch, keeps facebook#8901's regression tests (adapted to the leave-as-is expectation), and extends the same audit to the rest of the converter class: - lexical: LexicalNode.replace() now throws a slot-specific invariant ("use $setSlot on its host") before any mutation, mirroring the existing $removeFromParent guard for remove() on a slotted node — so a direct replace() of a slot value fails with an actionable error instead of the generic getParentOrThrow invariant. - @lexical/selection: $setBlocksType skips parentless blocks; the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list. - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root. Blocks *inside* a shadow-root slot value have parents and convert normally; covered by tests. Also documents the rule in the named-slots concepts page. ## Test plan Before: $setBlocksType converted a bare slot value by reassigning the slot ($setSlot inside the converter); $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" on slot values. After: the converters leave slot values as-is (and still convert blocks inside shadow-root slot values); replace() on a slot value throws a diagnostic pointing at $setSlot. facebook#8901's two regression tests updated to the leave-as-is expectation; new unit tests in LexicalSlot.test.ts, LexicalSetBlocksTypeSlots.test.ts, and LexicalFormatListSlots.test.ts.
$setBlocksType calls prevNode.replace(element, true) which internally calls getParentOrThrow(). For bare slot values (__parent is null, __slotHost is set), this throws. Detect bare slot values and use $setSlot to swap in the replacement element instead. Selection remapping is handled manually for the slot branch since replace() normally does this. Closes facebook#8894
Per maintainer review: $setBlocksType should not convert bare slot values, since the slot host owns the node type contract.
mayrang
force-pushed
the
fix/setblockstype-slot-value
branch
from
August 1, 2026 07:29
57b72b7 to
5445acc
Compare
Contributor
Author
|
Updated — the latest push just skips bare slot values ( |
etrepum
approved these changes
Aug 1, 2026
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 1, 2026
## Description Follow-up to facebook#8901 (fix for facebook#8894): $setBlocksType now skips slot values, but a direct LexicalNode.replace() on a slot value still fails with the generic "Expected node %s to have a parent" invariant from getParentOrThrow, which does not name the actual mistake. Throw a slot-specific invariant before any mutation instead, mirroring the existing $removeFromParent guard for remove() on a slotted node: the tree API cannot reassign a named slot; that is done with $setSlot on the host. ## Test plan Before: slotValue.replace(node) threw "Expected node %s to have a parent." After: it throws "replace: node %s is slotted into host %s; a slot value cannot be replaced through the tree API. Use $setSlot on its host to assign a replacement." New unit test in LexicalSlot.test.ts verifies the message and that nothing is mutated by the failed replace.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 1, 2026
…alue skip to the remaining block converters ## Description Follow-up to facebook#8901 (fix for facebook#8894): $setBlocksType now skips named-slot values, but the same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API. Audit and fix the rest, applying the same rule — a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is: - lexical: add a $hasSlotHost(node) predicate (the cheap boolean form of $getSlotHost, built on the internal $getSlotHostKey) so this recurring eligibility check is one helper instead of a per-site comparison, and a parentless slot value is never conflated with a detached node (which should keep failing loudly). $setBlocksType's guard switches to it. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (it previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. Blocks inside a shadow-root slot value have parents and convert normally (covered by tests). Documents the rule and the new predicate in the named-slots concepts page. ## Test plan Before: $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block slot value or a slotted list. After: they leave slot values as-is; blocks inside shadow-root slot values still convert. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a $hasSlotHost test in LexicalSlot.test.ts.
etrepum
approved these changes
Aug 1, 2026
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 1, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. - lexical: add a $hasSlotHost(node) predicate (the cheap boolean form of $getSlotHost, built on the internal $getSlotHostKey) so this recurring eligibility check is one helper instead of a per-site comparison, and a parentless slot value is never conflated with a detached node (which should keep failing loudly). $setBlocksType's guard switches to it. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. Blocks inside a shadow-root slot value have parents and convert normally. The rule and the new predicate are documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is, blocks inside shadow-root slot values still convert, and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus replace() and $hasSlotHost tests in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. Blocks inside a shadow-root slot value have parents and convert normally. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is, blocks inside shadow-root slot values still convert, and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. Blocks inside a shadow-root slot value have parents and convert normally. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is, blocks inside shadow-root slot values still convert, and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. Blocks inside a shadow-root slot value have parents and convert normally. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is, blocks inside shadow-root slot values still convert, and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. Blocks inside a shadow-root slot value have parents and convert normally. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is, blocks inside shadow-root slot values still convert, and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Aug 2, 2026
…alues in the remaining block converters and diagnose replace() on a slot value ## Description Follow-up to facebook#8901 (fix for facebook#8894), which made $setBlocksType skip named slot values. The same crash class exists in every other selection-driven block converter that resolves the current block and replaces it through the tree API, and a direct replace() of a slot value still fails with the unhelpful generic "Expected node %s to have a parent" invariant. This audits and fixes the rest, applying the same rule: a slot value's assignment is managed by the node or extension that owns the slot, so generic converters leave it as-is. The eligibility checks test the slot up-link ($getSlotHost) rather than parentless-ness, so a detached node is never conflated with a slot value and keeps failing loudly. - lexical: LexicalNode.replace() on a slot value now throws a slot-specific invariant before any mutation ("use $setSlot on its host to assign a replacement"), mirroring the existing $removeFromParent guard for remove() on a slotted node. - @lexical/selection: the deprecated $wrapNodes no-ops when the selection lives in a bare-block slot frame (it previously threw in the sibling walk); a shadow-root slot value still wraps its own subtree normally. - @lexical/list: $insertList skips slot values in its empty-block branch, its empty-list-item branch, and its walk-up list-type-change branch; $removeList skips a slotted top-level list (they previously threw or vacated the slot). - @lexical/markdown: audited, already safe — the element/multiline shortcut runners bail unless the block's parent is a root or shadow root, which excludes slot values. The rule is documented in the named-slots concepts page. ## Test plan ### Before $insertList, $removeList, and $wrapNodes threw "Expected node %s to have a parent" (or silently vacated a slot) when the selection was inside a bare-block named-slot value or a slotted list; replace() on a slot value threw the same generic invariant. ### After The converters leave slot values as-is and replace() on a slot value throws a diagnostic pointing at $setSlot. New unit tests in LexicalSetBlocksTypeSlots.test.ts and LexicalFormatListSlots.test.ts, plus a replace() test in LexicalSlot.test.ts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
$setBlocksTypecrashes with an invariant violation when the selection includes a bare slot value — a block node set directly via$setSlot(host, slotName, blockNode)whose__parentisnull. The crash hitsgetParentOrThrow()insideprevNode.replace().The fix skips bare slot values in the replacement loop: if
$getSlotHost(prevNode)returns a host, the node is a named slot value and$setBlocksTypeleaves it alone.Closes #8894
Test plan
Bare slot value is skipped (#8894)— creates aTestSlotHostwith a slot paragraph, calls$setBlocksTypeto convert to heading, verifies the paragraph stays unchanged.LexicalSelection.test.tsx.