From 4956ce287acbbcaf546ebf8b46cf5267190e3bb5 Mon Sep 17 00:00:00 2001 From: johnnyjacobs Date: Mon, 22 Sep 2025 12:00:12 -0700 Subject: [PATCH 1/2] Remove Paragraph Replacement in `HeadingNode` I'm not really sure why this is here - it seems both semantically incorrect in all the cases I've observed and also causes bugs in other Lexical functions, as if they keep a reference to the `HeadingNode` that is replaced, subsequent usage of that `HeadingNode` will cause errors. See a common example of this in `RangeSelection#insertNodes()` in the added test. --- packages/lexical-rich-text/src/index.ts | 5 - .../__tests__/unit/LexicalSelection.test.ts | 114 +++++++++++------- 2 files changed, 68 insertions(+), 51 deletions(-) diff --git a/packages/lexical-rich-text/src/index.ts b/packages/lexical-rich-text/src/index.ts index f9ba5e287d8..e1c3726be0a 100644 --- a/packages/lexical-rich-text/src/index.ts +++ b/packages/lexical-rich-text/src/index.ts @@ -376,11 +376,6 @@ export class HeadingNode extends ElementNode { const direction = this.getDirection(); newElement.setDirection(direction); this.insertAfter(newElement, restoreSelection); - if (anchorOffet === 0 && !this.isEmpty() && selection) { - const paragraph = $createParagraphNode(); - paragraph.select(); - this.replace(paragraph, true); - } return newElement; } diff --git a/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts b/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts index 7b3cf2109aa..dcf1ff2026b 100644 --- a/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts +++ b/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts @@ -13,6 +13,7 @@ import { ListItemNode, ListNode, } from '@lexical/list'; +import {$createHeadingNode} from '@lexical/rich-text'; import { $caretRangeFromSelection, $comparePointCaretNext, @@ -839,49 +840,6 @@ describe('LexicalSelection tests', () => { }); }); -describe('Regression tests for #6701', () => { - test('insertNodes fails an invariant when there is no Block ancestor', async () => { - class InlineElementNode extends ElementNode { - static clone(prevNode: InlineElementNode): InlineElementNode { - return new InlineElementNode(prevNode.__key); - } - static getType() { - return 'inline-element-node'; - } - static importJSON(serializedNode: SerializedElementNode) { - return new InlineElementNode().updateFromJSON(serializedNode); - } - isInline() { - return true; - } - createDOM() { - return document.createElement('span'); - } - updateDOM() { - return false; - } - } - const editor = createEditor({ - nodes: [InlineElementNode], - onError: (err) => { - throw err; - }, - }); - expect(() => - editor.update( - () => { - const textNode = $createTextNode('test'); - $getRoot().clear().append(new InlineElementNode().append(textNode)); - textNode.select().insertNodes([$createTextNode('more text')]); - }, - {discrete: true}, - ), - ).toThrow( - /Expected node TextNode of type text to have a block ElementNode ancestor/, - ); - }); -}); - describe('getNodes()', () => { initializeUnitTest((testEnv) => { let paragraphNode: ParagraphNode; @@ -1553,7 +1511,50 @@ describe('extract()', () => { }); }); -describe('Regression #7081', () => { +describe('Regression tests for #6701', () => { + test('insertNodes fails an invariant when there is no Block ancestor', async () => { + class InlineElementNode extends ElementNode { + static clone(prevNode: InlineElementNode): InlineElementNode { + return new InlineElementNode(prevNode.__key); + } + static getType() { + return 'inline-element-node'; + } + static importJSON(serializedNode: SerializedElementNode) { + return new InlineElementNode().updateFromJSON(serializedNode); + } + isInline() { + return true; + } + createDOM() { + return document.createElement('span'); + } + updateDOM() { + return false; + } + } + const editor = createEditor({ + nodes: [InlineElementNode], + onError: (err) => { + throw err; + }, + }); + expect(() => + editor.update( + () => { + const textNode = $createTextNode('test'); + $getRoot().clear().append(new InlineElementNode().append(textNode)); + textNode.select().insertNodes([$createTextNode('more text')]); + }, + {discrete: true}, + ), + ).toThrow( + /Expected node TextNode of type text to have a block ElementNode ancestor/, + ); + }); +}); + +describe('Regression tests for #7081', () => { initializeUnitTest((testEnv) => { test('Firefox selection & paste before linebreak', () => { testEnv.editor.update( @@ -1587,7 +1588,7 @@ describe('Regression #7081', () => { }); }); -describe('Regression #7173', () => { +describe('Regression tests for #7173', () => { initializeUnitTest((testEnv) => { test('Can insertNodes of multiple blocks with a target of an initial empty block and the entire next block', () => { testEnv.editor.update( @@ -1613,7 +1614,28 @@ describe('Regression #7173', () => { }); }); -describe('Regression #3181', () => { +describe('Regression tests for #7846', () => { + initializeUnitTest((testEnv) => { + test('insertNodes can insert in HeadingNode', async () => { + testEnv.editor.update( + () => { + const heading = $createHeadingNode('h1'); + const headingText = $createTextNode(' chyeah'); + $getRoot().clear().append(heading.append(headingText)); + const paragraph = $createParagraphNode(); + const paragraphText = $createTextNode('finna'); + const selection = headingText.selectStart(); + selection.insertNodes([paragraph.append(paragraphText)]); + expect($getRoot().getChildren()).toEqual([paragraph]); + expect($getRoot().getTextContent()).toEqual('finna chyeah'); + }, + {discrete: true}, + ); + }); + }); +}); + +describe('Regression tests for #3181', () => { initializeUnitTest((testEnv) => { test('Point.isBefore edge case with mixed TextNode & ElementNode and matching descendants', () => { testEnv.editor.update( From 4db8476ee22a426d8ea0825025ac921726fb97e8 Mon Sep 17 00:00:00 2001 From: johnnyjacobs Date: Mon, 22 Sep 2025 14:49:15 -0700 Subject: [PATCH 2/2] Re-Compute `firstBlock` in `RangeSelection#insertNodes` if Obsoleted Things like `insertParagraph` can remove `firstBlock` from the editor state, so re-compute it in those cases. --- packages/lexical-rich-text/src/index.ts | 5 +++++ packages/lexical/src/LexicalSelection.ts | 17 +++++++++++------ .../src/__tests__/unit/LexicalSelection.test.ts | 1 - 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/lexical-rich-text/src/index.ts b/packages/lexical-rich-text/src/index.ts index e1c3726be0a..f9ba5e287d8 100644 --- a/packages/lexical-rich-text/src/index.ts +++ b/packages/lexical-rich-text/src/index.ts @@ -376,6 +376,11 @@ export class HeadingNode extends ElementNode { const direction = this.getDirection(); newElement.setDirection(direction); this.insertAfter(newElement, restoreSelection); + if (anchorOffet === 0 && !this.isEmpty() && selection) { + const paragraph = $createParagraphNode(); + paragraph.select(); + this.replace(paragraph, true); + } return newElement; } diff --git a/packages/lexical/src/LexicalSelection.ts b/packages/lexical/src/LexicalSelection.ts index f0419396701..69ee038927c 100644 --- a/packages/lexical/src/LexicalSelection.ts +++ b/packages/lexical/src/LexicalSelection.ts @@ -1346,7 +1346,7 @@ export class RangeSelection implements BaseSelection { const firstPoint = this.isBackward() ? this.focus : this.anchor; const firstNode = firstPoint.getNode(); - const firstBlock = $getAncestor(firstNode, INTERNAL_$isBlock); + let firstBlock = $getAncestor(firstNode, INTERNAL_$isBlock); const last = nodes[nodes.length - 1]!; @@ -1383,17 +1383,22 @@ export class RangeSelection implements BaseSelection { const blocksParent = $wrapInlineNodes(nodes); const nodeToSelect = blocksParent.getLastDescendant()!; const blocks = blocksParent.getChildren(); + const shouldInsert = !$isElementNode(firstBlock) || !firstBlock.isEmpty(); + const insertedParagraph = shouldInsert ? this.insertParagraph() : null; + const lastToInsert: LexicalNode | undefined = blocks[blocks.length - 1]; + let firstToInsert: LexicalNode | undefined = blocks[0]; + if (!firstBlock || !firstBlock.isAttached()) { + firstBlock = $getAncestor( + firstNode, + INTERNAL_$isBlock, + ).getPreviousSibling(); + } const isMergeable = (node: LexicalNode): node is ElementNode => $isElementNode(node) && INTERNAL_$isBlock(node) && !node.isEmpty() && $isElementNode(firstBlock) && (!firstBlock.isEmpty() || firstBlock.canMergeWhenEmpty()); - - const shouldInsert = !$isElementNode(firstBlock) || !firstBlock.isEmpty(); - const insertedParagraph = shouldInsert ? this.insertParagraph() : null; - const lastToInsert: LexicalNode | undefined = blocks[blocks.length - 1]; - let firstToInsert: LexicalNode | undefined = blocks[0]; if (isMergeable(firstToInsert)) { invariant( $isElementNode(firstBlock), diff --git a/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts b/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts index dcf1ff2026b..afce5b3ce5e 100644 --- a/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts +++ b/packages/lexical/src/__tests__/unit/LexicalSelection.test.ts @@ -1626,7 +1626,6 @@ describe('Regression tests for #7846', () => { const paragraphText = $createTextNode('finna'); const selection = headingText.selectStart(); selection.insertNodes([paragraph.append(paragraphText)]); - expect($getRoot().getChildren()).toEqual([paragraph]); expect($getRoot().getTextContent()).toEqual('finna chyeah'); }, {discrete: true},