From f5272528d46e93d39ce915cb23700e10eb56dd43 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 22:34:06 -0700 Subject: [PATCH 1/2] [lexical-list] Bug Fix: Renumber a list when an item starts or stops wrapping a nested list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Closes #9661. An item that only wraps a nested list does not count in its list's numbering (`updateChildrenListItemValue`). When the nested list's last item is removed, the nested list goes and the wrapper item stays, empty, and now counts; when a list is appended to an empty item, the item stops counting. In both cases only the item is written, not its list, so the list's transform, which renumbers, did not run: the items after the item kept stale values (`1, 1` instead of `1, 2`), and a split there (paragraphs inserted at the empty item) started the next list from the stale value. `ListItemNode`'s transform now compares the item with its previous version: where it has started or stopped wrapping a nested list since the last update, its list renumbers its items. Nothing else changes: a value an app sets on an item directly is left as before (until the list is written), and an item that keeps wrapping its nested list leaves the list unwritten. ## Test plan `RenumberOnNestedListRemoval.test.ts`: - the issue's repro: removing the only item of a nested list renumbers the items after it, and a split there starts the next list at 2; - several items after it, in a list that starts at 5; - the last item of a nested list that has two (the first removal changes nothing, the second renumbers); - an empty item that comes to hold only a nested list stops counting; - bullet and check lists; - controls: the same split with up-to-date values; a removal that leaves the item wrapping its nested list leaves the outer list unwritten; a value an app sets on an item is not renumbered by this. ### Before main (c0c7e426): ``` Tests 6 failed | 2 passed (8) × removing the only item of a nested list renumbers the items after it expected [ { start: 1, values: [ 1, 1 ] } ] to deeply equal [ { start: 1, values: [ 1, 2 ] } ] ``` ### After ``` $ vitest --run --project unit packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts Tests 9 passed (9) ``` All unit tests: 24,726 passed, 421 expected fail, 0 failed. `pnpm run ci-check` passes. --- .changeset/list-renumber-nested-removal.md | 7 + .../lexical-list/src/LexicalListItemNode.ts | 26 +- .../unit/RenumberOnNestedListRemoval.test.ts | 237 ++++++++++++++++++ 3 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 .changeset/list-renumber-nested-removal.md create mode 100644 packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts diff --git a/.changeset/list-renumber-nested-removal.md b/.changeset/list-renumber-nested-removal.md new file mode 100644 index 00000000000..4019f43bf5e --- /dev/null +++ b/.changeset/list-renumber-nested-removal.md @@ -0,0 +1,7 @@ +--- +'@lexical/list': patch +--- + +[fix] Removing the last item of a nested list renumbers the items after its wrapper item, so a split there continues the numbering + +Verify: in a numbered list "ff", "gg hh", indent "ff", then remove it: "gg hh" reads 2. (It read 1, and inserting paragraphs at the empty item started the list after them at 1.) diff --git a/packages/lexical-list/src/LexicalListItemNode.ts b/packages/lexical-list/src/LexicalListItemNode.ts index 5c49dd4ead0..903ca479df3 100644 --- a/packages/lexical-list/src/LexicalListItemNode.ts +++ b/packages/lexical-list/src/LexicalListItemNode.ts @@ -55,7 +55,12 @@ import { } from 'lexical'; import {$createListNode, $isListNode, type ListType} from './'; -import {$handleIndent, $handleOutdent, mergeLists} from './formatList'; +import { + $handleIndent, + $handleOutdent, + mergeLists, + updateChildrenListItemValue, +} from './formatList'; import {GENERATED_LISTITEM} from './LexicalListGeneratedJSON'; import {ListNode} from './LexicalListNode'; import {$getNewListStart, $isNestedListNode} from './utils'; @@ -192,6 +197,25 @@ export class ListItemNode extends ElementNode { if (parent.getListType() !== 'check' && node.getChecked() != null) { node.setChecked(undefined); } + // An item that only wraps a nested list does not count in the + // numbering (see updateChildrenListItemValue). It can start or stop + // wrapping one without its list being written (its nested list's + // last item removed, a list appended to it while empty), and the + // list's own transform, which renumbers, does not run then: where + // the item has started or stopped wrapping a nested list since the + // last update, the list renumbers its items. Why: the items after + // it kept stale values, and a split there started the next list + // from them (#9661). + const prevMap = $getEditor().getEditorState()._nodeMap; + const prev = prevMap.get(node.getKey()); + if ( + $isListItemNode(prev) && + $isListNode( + prev.__first === null ? null : prevMap.get(prev.__first), + ) !== $isListNode(node.getFirstChild()) + ) { + updateChildrenListItemValue(parent); + } } else if (parent) { const newParent = node.createParentElementNode(); invariant( diff --git a/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts b/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts new file mode 100644 index 00000000000..5ca05ad6fa4 --- /dev/null +++ b/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts @@ -0,0 +1,237 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +import { + $createListItemNode, + $createListNode, + $isListItemNode, + $isListNode, + ListItemNode, + ListNode, + type ListType, + registerList, +} from '@lexical/list'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $setSelection, + createEditor, + type ElementNode, + type LexicalEditor, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +// #9661: an item that only wraps a nested list does not count in its list's +// numbering. When it stops or starts wrapping one (its nested list's last +// item removed, a list appended to an empty item), the list itself is not +// written, so its transform did not renumber the items after the item: they +// kept stale values, and a split there started the next list from them. + +function build(): LexicalEditor { + const editor = createEditor({ + namespace: 'renumber', + nodes: [ListNode, ListItemNode], + onError: e => { + throw e; + }, + }); + registerList(editor); + return editor; +} + +function run(editor: LexicalEditor, fn: () => void): void { + editor.update(fn, {discrete: true}); +} + +/** Each top-level list's item values, and its start. */ +function lists(editor: LexicalEditor): {start: number; values: number[]}[] { + return editor.getEditorState().read(() => + $getRoot() + .getChildren() + .filter($isListNode) + .map(list => ({ + start: list.getStart(), + values: list + .getChildren() + .filter($isListItemNode) + .map(item => item.getValue()), + })), + ); +} + +function $list(): ListNode { + return $getRoot().getFirstChildOrThrow() as ListNode; +} + +/** A list of `texts`, with the first item indented into a nested list. */ +function indentedFirst( + editor: LexicalEditor, + type: ListType, + texts: string[], +): void { + run(editor, () => { + $getRoot().append( + $createListNode(type).append( + ...texts.map(text => + $createListItemNode().append($createTextNode(text)), + ), + ), + $createParagraphNode().append($createTextNode('end')), + ); + }); + run(editor, () => { + ($list().getFirstChildOrThrow() as ListItemNode).setIndent(1); + }); +} + +function $removeOnlyNestedItem(): void { + const wrapper = $list().getFirstChildOrThrow() as ElementNode; + const nested = wrapper.getFirstChildOrThrow() as ElementNode; + nested.getFirstChildOrThrow().remove(); +} + +function $insertParagraphsAtFirstItem(): void { + const empty = $list().getFirstChildOrThrow() as ElementNode; + const selection = $createRangeSelection(); + selection.anchor.set(empty.getKey(), 0, 'element'); + selection.focus.set(empty.getKey(), 0, 'element'); + $setSelection(selection); + selection.insertNodes([ + $createParagraphNode().append($createTextNode('p')), + $createParagraphNode().append($createTextNode('q')), + ]); +} + +describe('registerList renumbers when an item stops or starts wrapping a nested list (#9661)', () => { + test('removing the only item of a nested list renumbers the items after it', () => { + const editor = build(); + indentedFirst(editor, 'number', ['ff', 'gg hh']); + expect(lists(editor)).toEqual([{start: 1, values: [1, 1]}]); + run(editor, $removeOnlyNestedItem); + // The empty item now counts, so "gg hh" is item 2. + expect(lists(editor)).toEqual([{start: 1, values: [1, 2]}]); + // A split there continues the numbering in the list after it. + run(editor, $insertParagraphsAtFirstItem); + expect(lists(editor).map(l => l.start)).toEqual([1, 2]); + }); + + test('control: the same split when the values are up to date', () => { + const editor = build(); + run(editor, () => { + $getRoot().append( + $createListNode('number').append( + $createListItemNode(), + $createListItemNode().append($createTextNode('gg hh')), + ), + $createParagraphNode().append($createTextNode('end')), + ); + }); + expect(lists(editor)).toEqual([{start: 1, values: [1, 2]}]); + run(editor, $insertParagraphsAtFirstItem); + expect(lists(editor).map(l => l.start)).toEqual([1, 2]); + }); + + test('several items after it, and a list that starts at 5', () => { + const editor = build(); + indentedFirst(editor, 'number', ['a', 'b', 'c', 'd']); + run(editor, () => $list().setStart(5)); + expect(lists(editor)).toEqual([{start: 5, values: [5, 5, 6, 7]}]); + run(editor, $removeOnlyNestedItem); + expect(lists(editor)).toEqual([{start: 5, values: [5, 6, 7, 8]}]); + }); + + test('the last item of a nested list that has two', () => { + const editor = build(); + indentedFirst(editor, 'number', ['a', 'b', 'c']); + run(editor, () => { + // Indent "b" too: the nested list holds "a" and "b". + ($list().getChildAtIndex(1) as ListItemNode).setIndent(1); + }); + expect(lists(editor)).toEqual([{start: 1, values: [1, 1]}]); + run(editor, () => { + const wrapper = $list().getFirstChildOrThrow() as ElementNode; + const nested = wrapper.getFirstChildOrThrow() as ElementNode; + nested.getLastChildOrThrow().remove(); + }); + // The wrapper still wraps a nested list ("a"): nothing changes. + expect(lists(editor)).toEqual([{start: 1, values: [1, 1]}]); + run(editor, $removeOnlyNestedItem); + expect(lists(editor)).toEqual([{start: 1, values: [1, 2]}]); + }); + + test('an empty item that comes to hold only a nested list stops counting', () => { + const editor = build(); + run(editor, () => { + $getRoot().append( + $createListNode('number').append( + $createListItemNode().append($createTextNode('a')), + $createListItemNode(), + $createListItemNode().append($createTextNode('c')), + ), + ); + }); + expect(lists(editor)).toEqual([{start: 1, values: [1, 2, 3]}]); + run(editor, () => { + ($list().getChildAtIndex(1) as ListItemNode).append( + $createListNode('number').append( + $createListItemNode().append($createTextNode('b')), + ), + ); + }); + expect(lists(editor)).toEqual([{start: 1, values: [1, 2, 2]}]); + }); + + test('a removal that leaves the item wrapping its nested list leaves the outer list unwritten', () => { + const editor = build(); + indentedFirst(editor, 'number', ['a', 'b', 'c']); + run(editor, () => { + ($list().getChildAtIndex(1) as ListItemNode).setIndent(1); + }); + const outer = editor.getEditorState().read(() => $list().getKey()); + let outerWritten = false; + const remove = editor.registerUpdateListener(({dirtyElements}) => { + outerWritten = outerWritten || dirtyElements.get(outer) === true; + }); + run(editor, () => { + // "a" is removed from the nested list, which still holds "b". + const wrapper = $list().getFirstChildOrThrow() as ElementNode; + const nested = wrapper.getFirstChildOrThrow() as ElementNode; + nested.getFirstChildOrThrow().remove(); + }); + remove(); + expect(lists(editor)).toEqual([{start: 1, values: [1, 1]}]); + expect(outerWritten).toBe(false); + }); + + test('control: a value an app sets on an item is not renumbered by this', () => { + const editor = build(); + run(editor, () => { + $getRoot().append( + $createListNode('number').append( + $createListItemNode().append($createTextNode('a')), + $createListItemNode().append($createTextNode('b')), + ), + ); + }); + run(editor, () => { + ($list().getFirstChildOrThrow() as ListItemNode).setValue(7); + }); + expect(lists(editor)).toEqual([{start: 1, values: [7, 2]}]); + }); + + for (const type of ['bullet', 'check'] as const) { + test(`${type} lists renumber the same way`, () => { + const editor = build(); + indentedFirst(editor, type, ['ff', 'gg hh']); + run(editor, $removeOnlyNestedItem); + expect(lists(editor)).toEqual([{start: 1, values: [1, 2]}]); + }); + } +}); From bf7d979278826e7f66bf2a37a4835c41cb856de9 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 22:50:27 -0700 Subject: [PATCH 2/2] [lexical-list] Bug Fix: Renumber once per update from the first changed item, keeping values an app set ## Description Review of the first commit found two flaws in the renumbering: - It compared the item with the committed state and renumbered the whole list, so it also overwrote a value an app set on the item in the same update, and an app transform that keeps a value (`setValue(7)`) fought it until Lexical threw and rolled the update back. - Each changed item renumbered the whole list: emptying N wrappers in one update cost N renumberings (512 wrappers: 523,776 value reads). Now an item that started or stopped wrapping a nested list renumbers only where the item after it is out of step, and the renumbering (`$renumberAfterWrapChange`) starts at the first changed item of the list: the items before it and that item keep their values, an item whose value changed in this update keeps it (the items after it count on from it), and one pass leaves the list consistent, so the other changed items find nothing to do. ## Test plan `RenumberOnNestedListRemoval.test.ts` adds: a value an app sets in the same update as the removal stays (7, and the next item 8); an app transform that keeps a value does not conflict (no error, 7 and 8); emptying 64, 128 and 256 wrappers in one update, first to last and last to first, reads values a linear number of times. The four fail on the previous head; all 13 pass here. All unit tests: 24,730 passed, 421 expected fail, 0 failed. `pnpm run ci-check` passes. --- .../lexical-list/src/LexicalListItemNode.ts | 86 +++++++++++++++---- .../unit/RenumberOnNestedListRemoval.test.ts | 84 ++++++++++++++++++ 2 files changed, 152 insertions(+), 18 deletions(-) diff --git a/packages/lexical-list/src/LexicalListItemNode.ts b/packages/lexical-list/src/LexicalListItemNode.ts index 903ca479df3..62b43123640 100644 --- a/packages/lexical-list/src/LexicalListItemNode.ts +++ b/packages/lexical-list/src/LexicalListItemNode.ts @@ -55,12 +55,7 @@ import { } from 'lexical'; import {$createListNode, $isListNode, type ListType} from './'; -import { - $handleIndent, - $handleOutdent, - mergeLists, - updateChildrenListItemValue, -} from './formatList'; +import {$handleIndent, $handleOutdent, mergeLists} from './formatList'; import {GENERATED_LISTITEM} from './LexicalListGeneratedJSON'; import {ListNode} from './LexicalListNode'; import {$getNewListStart, $isNestedListNode} from './utils'; @@ -201,20 +196,23 @@ export class ListItemNode extends ElementNode { // numbering (see updateChildrenListItemValue). It can start or stop // wrapping one without its list being written (its nested list's // last item removed, a list appended to it while empty), and the - // list's own transform, which renumbers, does not run then: where + // list's own transform, which renumbers, does not run then. Where // the item has started or stopped wrapping a nested list since the - // last update, the list renumbers its items. Why: the items after - // it kept stale values, and a split there started the next list - // from them (#9661). + // last update and the item after it has another value than this + // item gives it, the list's items are renumbered once (see + // $renumberAfterWrapChange). Why: the items after it kept stale + // values, and a split there started the next list from them + // (#9661). const prevMap = $getEditor().getEditorState()._nodeMap; - const prev = prevMap.get(node.getKey()); - if ( - $isListItemNode(prev) && - $isListNode( - prev.__first === null ? null : prevMap.get(prev.__first), - ) !== $isListNode(node.getFirstChild()) - ) { - updateChildrenListItemValue(parent); + const wraps = $isListNode(node.getFirstChild()); + if ($wrapChanged(node, prevMap)) { + const next = node.getNextSibling(); + if ( + $isListItemNode(next) && + next.getValue() !== node.getValue() + (wraps ? 0 : 1) + ) { + $renumberAfterWrapChange(parent, prevMap); + } } } else if (parent) { const newParent = node.createParentElementNode(); @@ -883,6 +881,58 @@ export function $createListItemNode(checked?: boolean): ListItemNode { * @param node - The node to be checked. * @returns true if the node is a ListItemNode, false otherwise. */ +/** Whether `item` started or stopped wrapping a nested list since `prevMap`. */ +function $wrapChanged( + item: ListItemNode, + prevMap: Map, +): boolean { + const prev = prevMap.get(item.getKey()); + return ( + $isListItemNode(prev) && + $isListNode(prev.__first === null ? null : prevMap.get(prev.__first)) !== + $isListNode(item.getFirstChild()) + ); +} + +/** + * Renumbers the items of `list` after the first of them that started or + * stopped wrapping a nested list (see ListItemNode's transform), counting on + * from that item's value as updateChildrenListItemValue counts. The items + * before it and that item keep their values, and so does an item whose value + * changed in this update (an app set it), the items after it counting on from + * it. Why: an app that keeps a value (a transform that sets it) never + * conflicts with this. One pass leaves the list consistent, so the other + * changed items of the update find nothing to do: the cost is linear in the + * list, however many items changed. + */ +function $renumberAfterWrapChange( + list: ListNode, + prevMap: Map, +): void { + let started = false; + let value = list.getStart(); + for (const child of list.getChildren()) { + if (!$isListItemNode(child)) { + continue; + } + const current = child.getValue(); + if (!started) { + started = $wrapChanged(child, prevMap); + value = current; + } else { + const prev = prevMap.get(child.getKey()); + if ($isListItemNode(prev) && prev.__value !== current) { + value = current; + } else if (current !== value) { + child.setValue(value); + } + } + if (!$isListNode(child.getFirstChild())) { + value++; + } + } +} + export function $isListItemNode( node: LexicalNode | null | undefined, ): node is ListItemNode { diff --git a/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts b/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts index 5ca05ad6fa4..578f0383d4b 100644 --- a/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts +++ b/packages/lexical-list/src/__tests__/unit/RenumberOnNestedListRemoval.test.ts @@ -234,4 +234,88 @@ describe('registerList renumbers when an item stops or starts wrapping a nested expect(lists(editor)).toEqual([{start: 1, values: [1, 2]}]); }); } + + test('a value an app sets in the same update as the removal stays', () => { + const editor = build(); + indentedFirst(editor, 'number', ['a', 'b']); + run(editor, () => { + $removeOnlyNestedItem(); + ($list().getFirstChildOrThrow() as ListItemNode).setValue(7); + }); + // The empty item keeps 7, and "b" counts on from it. + expect(lists(editor)).toEqual([{start: 1, values: [7, 8]}]); + }); + + test('an app transform that keeps a value does not conflict with the renumbering', () => { + const editor = build(); + indentedFirst(editor, 'number', ['a', 'b']); + const errors: string[] = []; + editor._onError = e => { + errors.push(e.message); + }; + const key = editor + .getEditorState() + .read(() => $list().getFirstChildOrThrow().getKey()); + const unregister = editor.registerNodeTransform(ListItemNode, node => { + if (node.getKey() === key && node.getValue() !== 7) { + node.setValue(7); + } + }); + run(editor, () => {}); + run(editor, $removeOnlyNestedItem); + unregister(); + expect(errors).toEqual([]); + expect(lists(editor)).toEqual([{start: 1, values: [7, 8]}]); + }); + + // Many wrappers emptied in one update: one renumbering, whatever order the + // items' transforms run in. + for (const order of ['first to last', 'last to first'] as const) { + test(`removals in many wrappers at once cost linear work (${order})`, () => { + const counts: number[] = []; + for (const n of [64, 128, 256]) { + const editor = build(); + run(editor, () => { + $getRoot().append( + $createListNode('number').append( + ...Array.from({length: n}, (_, i) => + $createListItemNode().append( + $createListNode('number').append( + $createListItemNode().append($createTextNode(`x${i}`)), + ), + ), + ), + ), + ); + }); + let reads = 0; + const original = ListItemNode.prototype.getValue; + ListItemNode.prototype.getValue = function (this: ListItemNode) { + reads++; + return original.call(this); + }; + try { + run(editor, () => { + const wrappers = $list().getChildren() as ElementNode[]; + if (order === 'last to first') { + wrappers.reverse(); + } + for (const wrapper of wrappers) { + (wrapper.getFirstChildOrThrow() as ElementNode) + .getFirstChildOrThrow() + .remove(); + } + }); + } finally { + ListItemNode.prototype.getValue = original; + } + expect(lists(editor)[0].values).toEqual( + Array.from({length: n}, (_, i) => i + 1), + ); + counts.push(reads); + } + expect(counts[0]).toBeGreaterThan(0); + expect(counts[2]).toBeLessThan(counts[0] * 4 * 1.5); + }); + } });