From 5104cc116a2c62dc95bc4693a995590245df8c67 Mon Sep 17 00:00:00 2001 From: Supratim Sarkar Date: Thu, 2 Jul 2026 16:02:57 +0530 Subject: [PATCH 1/3] fix(core): recursively convert nested lists when toggling list type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit toggleList's 'change list type' branch called tr.setNodeMarkup only on the top-level list node found by findParentNode. When the selection spanned a list containing nested sublists (e.g. a bulletList with nested bulletList items), only the outer list was converted — nested sublists kept their original type, breaking the visual nesting hierarchy. Fixes #7970 Add convertNestedLists() which walks the list subtree after the top-level conversion and converts any nested list node still on the original type. setNodeMarkup only changes a node's type/attrs (not its size), so captured positions remain valid without re-resolving after each conversion. Adds regression tests covering 2-level and 3-level nested list toggling, plus a changeset for the @tiptap/core patch bump. --- .../fix-toggle-list-nested-structure.md | 5 + packages/core/src/commands/toggleList.ts | 40 +++++++ .../toggleListNestedStructure.spec.ts | 107 ++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 .changeset/fix-toggle-list-nested-structure.md create mode 100644 packages/extension-list/__tests__/toggleListNestedStructure.spec.ts diff --git a/.changeset/fix-toggle-list-nested-structure.md b/.changeset/fix-toggle-list-nested-structure.md new file mode 100644 index 0000000000..3fb0ee7ea5 --- /dev/null +++ b/.changeset/fix-toggle-list-nested-structure.md @@ -0,0 +1,5 @@ +--- +"@tiptap/core": patch +--- + +Fix `toggleList` (and the `toggleBulletList`/`toggleOrderedList`/`toggleTaskList` commands built on it) only converting the outermost list node when changing list type on a selection that spans nested sublists. Previously `tr.setNodeMarkup` was called on the top-level list only, leaving nested sublists on their original type and breaking the visual nesting hierarchy (e.g. converting a bullet list to an ordered list left nested bullet sublists as bullets). Nested list nodes of the original type are now recursively converted along with the top-level list. diff --git a/packages/core/src/commands/toggleList.ts b/packages/core/src/commands/toggleList.ts index d040d0c266..45d484cdef 100644 --- a/packages/core/src/commands/toggleList.ts +++ b/packages/core/src/commands/toggleList.ts @@ -58,6 +58,43 @@ const joinListBackwards = (tr: Transaction, listType: NodeType): boolean => { return true } +/** + * `tr.setNodeMarkup` only changes the type of the single node at the given + * position. When a list contains nested sublists (e.g. a bulletList whose + * list items contain further bulletList nodes), only the outermost list node + * was being converted, leaving nested sublists on their original type and + * breaking the visual hierarchy. This walks the whole list subtree and + * converts every nested list node that still has the original type. + */ +function convertNestedLists( + tr: Transaction, + listPos: number, + oldListType: NodeType, + newListType: NodeType, +): void { + const listNode = tr.doc.nodeAt(listPos) + + if (!listNode) { + return + } + + const positions: number[] = [] + + listNode.descendants((node, relativePos) => { + if (node.type === oldListType) { + positions.push(listPos + 1 + relativePos) + } + return true + }) + + // Convert deepest nodes first so earlier position lookups in this pass + // (captured before any mutation) remain valid — setNodeMarkup does not + // change node sizes, only its type/attrs, so positions never shift. + for (const pos of positions) { + tr.setNodeMarkup(pos, newListType) + } +} + const joinListForwards = (tr: Transaction, listType: NodeType): boolean => { const list = findParentNode(node => node.type === listType)(tr.selection) @@ -205,9 +242,12 @@ export const toggleList: RawCommands['toggleList'] = isList(currentList.node.type.name, extensions) && listType.validContent(currentList.node.content) ) { + const oldListType = currentList.node.type + return chain() .command(() => { tr.setNodeMarkup(currentList.pos, listType) + convertNestedLists(tr, currentList.pos, oldListType, listType) return true }) diff --git a/packages/extension-list/__tests__/toggleListNestedStructure.spec.ts b/packages/extension-list/__tests__/toggleListNestedStructure.spec.ts new file mode 100644 index 0000000000..1e2efaef3b --- /dev/null +++ b/packages/extension-list/__tests__/toggleListNestedStructure.spec.ts @@ -0,0 +1,107 @@ +import { Editor } from '@tiptap/core' +import Document from '@tiptap/extension-document' +import Paragraph from '@tiptap/extension-paragraph' +import Text from '@tiptap/extension-text' +import { afterEach, describe, expect, it } from 'vitest' + +import { BulletList, ListItem, OrderedList } from '../src/index.js' + +describe('toggleList preserves nested list structure', () => { + let editor: Editor + + afterEach(() => { + editor?.destroy() + }) + + it('converts nested bulletList to orderedList when toggling the whole selection', () => { + editor = new Editor({ + extensions: [Document, Paragraph, Text, ListItem, BulletList, OrderedList], + content: ` + + `, + }) + + editor.commands.selectAll() + editor.commands.toggleOrderedList() + + const json = editor.getJSON() + + expect(json.content).toHaveLength(1) + expect(json.content![0].type).toBe('orderedList') + + const outerItems = json.content![0].content! + expect(outerItems).toHaveLength(3) + + // The nested list inside "Item 2" must also have been converted — + // previously it kept its original bulletList type. + const nestedListNode = outerItems[1].content!.find(node => node.type === 'bulletList') + expect(nestedListNode).toBeUndefined() + + const nestedOrderedList = outerItems[1].content!.find(node => node.type === 'orderedList') + expect(nestedOrderedList).toBeDefined() + expect(nestedOrderedList!.content).toHaveLength(2) + }) + + it('converts a deeply nested (3-level) list hierarchy entirely', () => { + editor = new Editor({ + extensions: [Document, Paragraph, Text, ListItem, BulletList, OrderedList], + content: ` + + `, + }) + + editor.commands.selectAll() + editor.commands.toggleOrderedList() + + const findAllListTypes = (node: any, acc: string[] = []): string[] => { + if (node.type === 'bulletList' || node.type === 'orderedList') { + acc.push(node.type) + } + ;(node.content || []).forEach((child: any) => findAllListTypes(child, acc)) + return acc + } + + const json = editor.getJSON() + const listTypes = findAllListTypes({ type: 'doc', content: json.content }) + + expect(listTypes).toEqual(['orderedList', 'orderedList', 'orderedList']) + }) + + it('leaves a single-level list conversion working as before', () => { + editor = new Editor({ + extensions: [Document, Paragraph, Text, ListItem, BulletList, OrderedList], + content: '', + }) + + editor.commands.selectAll() + editor.commands.toggleOrderedList() + + const json = editor.getJSON() + + expect(json.content).toHaveLength(1) + expect(json.content![0].type).toBe('orderedList') + expect(json.content![0].content).toHaveLength(2) + }) +}) From 0f59293f0e9a389e345994e99ecec08a0f9d840b Mon Sep 17 00:00:00 2001 From: Supratim Sarkar Date: Fri, 3 Jul 2026 12:41:22 +0530 Subject: [PATCH 2/3] fix(core): stop clearNodes from dropping deeply nested content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @arnaugomez pointed out issue #7970 also names toggleTaskList, which this PR's first commit didn't cover — taskList wasn't tested at all. taskList has a different content model (taskItem+) than bulletList/orderedList (listItem+), so taskList <-> bulletList/ orderedList conversions go through toggleList's wrapInList + clearNodes fallback branch, not the setNodeMarkup 'change list type' branch the first commit fixed. That fallback had a separate, more severe bug: clearNodes() lifted every non-text node found by nodesBetween in document order, including wrapper nodes like listItem/ taskItem, not just textblocks. Lifting an outer wrapper before its still-to-be-visited nested children were processed mutated the document mid-iteration and corrupted the positions those later callbacks relied on. For a taskList nested inside a taskItem inside a taskList, this ejected the nested item's paragraph out of the list entirely, orphaning it as a bare top-level paragraph sibling — outright content loss, not just a wrong list type. clearNodes() now only lifts textblocks, processed in reverse document order so a deeper node's liftTarget is computed against a still-intact ancestor chain instead of one already reshaped by an earlier sibling's lift in the same pass. Container nodes are never lifted independently. This fixes the content-loss bug for all three commands #7970 named (toggleBulletList, toggleOrderedList, toggleTaskList) and top-level type conversion for taskList cross-family toggles now works correctly. Known remaining gap: when a cross-family conversion has a NESTED sublist of the other family (e.g. a taskList nested inside what's becoming a bulletList), the nested sublist itself keeps its original type/item-node type — only the outer list converts. taskItem and listItem are different node types with incompatible content models, so converting the nested sublist requires reconstructing it (new list + item nodes, not just setNodeMarkup), which this commit does not attempt. Flagged as a known limitation for now rather than risking an under-tested deeper rewrite; content-loss (the severe part) is fixed. Tests: packages/core/__tests__/clearNodes.spec.ts covers the fixed command directly; packages/extension-list/__tests__/ toggleListTaskListCrossFamily.spec.ts covers the taskList toggle scenarios named in #7970. Full monorepo suite (1368 tests) passes. --- .../fix-clearnodes-nested-content-loss.md | 5 + packages/core/__tests__/clearNodes.spec.ts | 60 ++++++++++ packages/core/src/commands/clearNodes.ts | 60 +++++++--- .../toggleListTaskListCrossFamily.spec.ts | 109 ++++++++++++++++++ 4 files changed, 218 insertions(+), 16 deletions(-) create mode 100644 .changeset/fix-clearnodes-nested-content-loss.md create mode 100644 packages/core/__tests__/clearNodes.spec.ts create mode 100644 packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts diff --git a/.changeset/fix-clearnodes-nested-content-loss.md b/.changeset/fix-clearnodes-nested-content-loss.md new file mode 100644 index 0000000000..e766663e17 --- /dev/null +++ b/.changeset/fix-clearnodes-nested-content-loss.md @@ -0,0 +1,5 @@ +--- +"@tiptap/core": patch +--- + +Fix `clearNodes()` dropping content when clearing a doubly-nested structure (e.g. a list nested inside a list item nested inside another list, as produced by `toggleTaskList`/`toggleBulletList`/`toggleOrderedList` when the selection spans a cross-family list conversion such as taskList → bulletList). `clearNodes()` previously lifted every non-text node found in the selection, including wrapper nodes like `listItem`/`taskItem`, not just textblocks — lifting an outer wrapper before its still-to-be-visited nested children were processed mutated the document mid-iteration and corrupted the positions those later callbacks relied on, ejecting deeply nested content out of the list entirely instead of flattening it alongside its siblings. `clearNodes()` now only lifts textblocks, processed in reverse document order, and no longer independently lifts container nodes. diff --git a/packages/core/__tests__/clearNodes.spec.ts b/packages/core/__tests__/clearNodes.spec.ts new file mode 100644 index 0000000000..3a7904c10c --- /dev/null +++ b/packages/core/__tests__/clearNodes.spec.ts @@ -0,0 +1,60 @@ +import { Editor } from '@tiptap/core' +import Document from '@tiptap/extension-document' +import Paragraph from '@tiptap/extension-paragraph' +import Text from '@tiptap/extension-text' +import BulletList from '@tiptap/extension-list/bullet-list' +import ListItem from '@tiptap/extension-list/item' +import { afterEach, describe, expect, it } from 'vitest' + +// clearNodes() used to lift every non-text node found by nodesBetween in +// document order — including wrapper/container nodes like listItem, not +// just textblocks. Lifting an outer wrapper before its still-to-be-visited +// nested children were processed mutated the document mid-iteration and +// corrupted the mapped positions those later callbacks relied on. For a +// list nested two levels deep, this ejected the deepest paragraph out of +// the list entirely instead of flattening it alongside its siblings — +// outright content loss. +describe('clearNodes', () => { + let editor: Editor + + afterEach(() => { + editor?.destroy() + }) + + it('does not drop content when clearing a doubly-nested list', () => { + editor = new Editor({ + extensions: [Document, Paragraph, Text, ListItem, BulletList], + content: ` + + `, + }) + + editor.commands.selectAll() + editor.commands.clearNodes() + + expect(editor.getText()).toContain('Item 1') + expect(editor.getText()).toContain('Item 1-1') + }) + + it('turns a simple textblock into a paragraph', () => { + editor = new Editor({ + extensions: [Document, Paragraph, Text, ListItem, BulletList], + content: '', + }) + + editor.commands.selectAll() + editor.commands.clearNodes() + + const json = editor.getJSON() + expect(json.content).toHaveLength(1) + expect(json.content![0].type).toBe('paragraph') + expect(editor.getText()).toBe('Hello') + }) +}) diff --git a/packages/core/src/commands/clearNodes.ts b/packages/core/src/commands/clearNodes.ts index b765084670..d2e00fe5de 100644 --- a/packages/core/src/commands/clearNodes.ts +++ b/packages/core/src/commands/clearNodes.ts @@ -25,32 +25,60 @@ export const clearNodes: RawCommands['clearNodes'] = } ranges.forEach(({ $from, $to }) => { + // Collect textblock positions from the original, unmutated doc first. + // Wrapper/container nodes (list, listItem, taskList, taskItem, ...) are + // intentionally skipped — nodesBetween walks every non-text node in the + // range, and lifting those independently, on top of lifting their + // textblock children in the same pass, mutates the doc mid-iteration + // and invalidates the still-to-be-visited positions of other + // descendants. Lifting a textblock all the way out of its ancestor + // chain already removes now-empty wrappers as part of the same + // transform step, so wrappers never need to be lifted separately. + const textblockPositions: number[] = [] + state.doc.nodesBetween($from.pos, $to.pos, (node, pos) => { - if (node.type.isText) { - return + if (node.type.isTextblock) { + textblockPositions.push(pos) } + }) - const { doc, mapping } = tr - const $mappedFrom = doc.resolve(mapping.map(pos)) - const $mappedTo = doc.resolve(mapping.map(pos + node.nodeSize)) - const nodeRange = $mappedFrom.blockRange($mappedTo) + // Process in reverse document order. Lifting a deeper/later textblock + // first (while ancestor nodes further up the doc are still intact) + // lets `liftTarget` climb out of the full nesting chain in one shot. + // Processing forward instead — as this used to — meant an earlier, + // shallower textblock could get lifted first, changing its container's + // shape before a still-to-be-processed deeper sibling's liftTarget was + // computed, which could cap that later lift to a shallower depth than + // it should reach (e.g. a nested taskList inside a taskItem only + // getting lifted one level instead of all the way out). + textblockPositions + .slice() + .reverse() + .forEach(pos => { + const { doc, mapping } = tr + const $mappedFrom = doc.resolve(mapping.map(pos)) + const node = $mappedFrom.nodeAfter - if (!nodeRange) { - return - } + if (!node || !node.type.isTextblock) { + return + } - const targetLiftDepth = liftTarget(nodeRange) + const $mappedTo = doc.resolve(mapping.map(pos) + node.nodeSize) + const nodeRange = $mappedFrom.blockRange($mappedTo) - if (node.type.isTextblock) { + if (!nodeRange) { + return + } + + const targetLiftDepth = liftTarget(nodeRange) const { defaultType } = $mappedFrom.parent.contentMatchAt($mappedFrom.index()) tr.setNodeMarkup(nodeRange.start, defaultType) - } - if (targetLiftDepth || targetLiftDepth === 0) { - tr.lift(nodeRange, targetLiftDepth) - } - }) + if (targetLiftDepth || targetLiftDepth === 0) { + tr.lift(nodeRange, targetLiftDepth) + } + }) }) return true diff --git a/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts b/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts new file mode 100644 index 0000000000..c88fdeb164 --- /dev/null +++ b/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts @@ -0,0 +1,109 @@ +import { Editor } from '@tiptap/core' +import Document from '@tiptap/extension-document' +import Paragraph from '@tiptap/extension-paragraph' +import Text from '@tiptap/extension-text' +import { afterEach, describe, expect, it } from 'vitest' + +import { BulletList, ListItem, OrderedList, TaskItem, TaskList } from '../src/index.js' + +// taskList has a different content model (taskItem+) than bulletList/orderedList +// (listItem+), so cross-family conversions go through toggleList's wrapInList +// + clearNodes fallback branch rather than the setNodeMarkup "change list +// type" branch used for bulletList <-> orderedList. That fallback had a +// separate, more severe bug: clearNodes() lifted every non-text node in the +// selection (including list/listItem/taskList/taskItem wrapper nodes, not +// just textblocks) in document order, using positions mapped against a doc +// that earlier iterations in the same pass had already mutated. For a +// taskList nested inside a taskItem inside a taskList, this ejected the +// nested item's paragraph out of the list entirely, orphaning it as a bare +// top-level paragraph sibling — outright content loss, not just a wrong +// list type. +describe('toggleList with taskList (cross-family, nested content)', () => { + let editor: Editor + + afterEach(() => { + editor?.destroy() + }) + + const setup = (content: string) => { + editor = new Editor({ + extensions: [ + Document, + Paragraph, + Text, + ListItem, + BulletList, + OrderedList, + TaskItem.configure({ nested: true }), + TaskList, + ], + content, + }) + return editor + } + + const nestedTaskListDoc = ` + + ` + + const nestedBulletListDoc = ` + + ` + + it('does not drop nested content when converting nested taskList to bulletList', () => { + setup(nestedTaskListDoc) + editor.commands.selectAll() + editor.commands.toggleBulletList() + + // The critical invariant: no text is lost. Previously "Item 1-1" was + // ejected out of the list as an orphaned top-level paragraph. + expect(editor.getText()).toContain('Item 1') + expect(editor.getText()).toContain('Item 1-1') + + const json = editor.getJSON() + // Everything stays inside a single top-level list — nothing orphaned + // as a bare sibling paragraph outside of it. + expect(json.content).toHaveLength(1) + expect(json.content![0].type).toBe('bulletList') + }) + + it('does not drop nested content when converting nested bulletList to taskList', () => { + setup(nestedBulletListDoc) + editor.commands.selectAll() + editor.commands.toggleTaskList() + + expect(editor.getText()).toContain('Item 1') + expect(editor.getText()).toContain('Item 1-1') + + const json = editor.getJSON() + expect(json.content).toHaveLength(1) + expect(json.content![0].type).toBe('taskList') + }) + + it('does not drop nested content when converting nested taskList to orderedList', () => { + setup(nestedTaskListDoc) + editor.commands.selectAll() + editor.commands.toggleOrderedList() + + expect(editor.getText()).toContain('Item 1') + expect(editor.getText()).toContain('Item 1-1') + + const json = editor.getJSON() + expect(json.content).toHaveLength(1) + expect(json.content![0].type).toBe('orderedList') + }) +}) From 990312ccb0d19f133ed35f0054b8dd9041050d6b Mon Sep 17 00:00:00 2001 From: Supratim Sarkar Date: Mon, 6 Jul 2026 12:30:14 +0530 Subject: [PATCH 3/3] fix(core): rebuild cross-family nested sublists when toggling list type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap flagged in the previous commit: nested sublists of a DIFFERENT list family (taskList inside what's becoming a bulletList, or vice versa) kept their original type because setNodeMarkup can only swap a node's own type — taskItem and listItem are incompatible node types, so the subtree has to be recreated node by node. convertNestedLists now classifies each nested sublist: - same family (bulletList <-> orderedList): wrapper type swapped in place via setNodeMarkup, walk keeps descending (unchanged behavior) - cross family: the whole subtree is rebuilt via rebuildListNode(), which recreates every item as the target item type and recursively converts deeper nested lists, then replaces the original in one size-preserving replaceWith (same child structure, only wrapper types change, so all positions collected up front stay valid) rebuildListNode() validates the rebuilt content against the target content models and bails (leaving the original untouched) rather than ever producing an invalid document. Family-specific item attributes with no equivalent on the target (taskItem's checked) are dropped — a bullet list has no checked state to carry them. The wrapInList fallback branches (taken for cross-family toggles of the top-level list) now also run the converter over every target-type list overlapping the selection, so sublists that survive inside the newly wrapped items get converted too. Tests upgraded from content-preservation-only to full-conversion assertions, plus new cases: cross-family sublist inside a same-family toggle, checked-state round-trip, and 3-level deep nesting. All 6 cross-family tests fail without this commit's source change (verified by reverting only toggleList.ts) and pass with it. Full monorepo suite: 1371 tests pass. --- .../fix-toggle-list-nested-structure.md | 2 +- packages/core/src/commands/toggleList.ts | 168 +++++++++++++++--- .../toggleListTaskListCrossFamily.spec.ts | 143 +++++++++++++-- 3 files changed, 275 insertions(+), 38 deletions(-) diff --git a/.changeset/fix-toggle-list-nested-structure.md b/.changeset/fix-toggle-list-nested-structure.md index 3fb0ee7ea5..4f4561e67c 100644 --- a/.changeset/fix-toggle-list-nested-structure.md +++ b/.changeset/fix-toggle-list-nested-structure.md @@ -2,4 +2,4 @@ "@tiptap/core": patch --- -Fix `toggleList` (and the `toggleBulletList`/`toggleOrderedList`/`toggleTaskList` commands built on it) only converting the outermost list node when changing list type on a selection that spans nested sublists. Previously `tr.setNodeMarkup` was called on the top-level list only, leaving nested sublists on their original type and breaking the visual nesting hierarchy (e.g. converting a bullet list to an ordered list left nested bullet sublists as bullets). Nested list nodes of the original type are now recursively converted along with the top-level list. +Fix `toggleList` (and the `toggleBulletList`/`toggleOrderedList`/`toggleTaskList` commands built on it) only converting the outermost list node when changing list type on a selection that spans nested sublists. Previously `tr.setNodeMarkup` was called on the top-level list only, leaving nested sublists on their original type and breaking the visual nesting hierarchy (e.g. converting a bullet list to an ordered list left nested bullet sublists as bullets). Nested sublists are now converted along with the top-level list: same-family sublists (bulletList ↔ orderedList) have their wrapper type swapped in place, and cross-family sublists (taskList ↔ bulletList/orderedList, whose item node types are incompatible) are rebuilt node by node — including the item nodes themselves — so a `toggleTaskList`/`toggleBulletList` across families converts the entire nesting hierarchy. Family-specific item attributes with no equivalent on the target family (e.g. taskItem's `checked`) are dropped in the conversion. diff --git a/packages/core/src/commands/toggleList.ts b/packages/core/src/commands/toggleList.ts index 45d484cdef..047c278290 100644 --- a/packages/core/src/commands/toggleList.ts +++ b/packages/core/src/commands/toggleList.ts @@ -1,4 +1,5 @@ -import type { NodeType } from '@tiptap/pm/model' +import type { Node as ProseMirrorNode, NodeType } from '@tiptap/pm/model' +import { Fragment } from '@tiptap/pm/model' import type { Transaction } from '@tiptap/pm/state' import { TextSelection } from '@tiptap/pm/state' import { canJoin } from '@tiptap/pm/transform' @@ -6,7 +7,7 @@ import { canJoin } from '@tiptap/pm/transform' import { findParentNode } from '../helpers/findParentNode.js' import { getNodeType } from '../helpers/getNodeType.js' import { isList } from '../helpers/isList.js' -import type { RawCommands } from '../types.js' +import type { Extensions, RawCommands } from '../types.js' /** * Normalise a list type attribute for comparison. @@ -58,19 +59,90 @@ const joinListBackwards = (tr: Transaction, listType: NodeType): boolean => { return true } +/** + * Rebuild a list node from a different item family (e.g. a taskList whose + * items are taskItem nodes) into `listType` with `itemType` items, + * recursively converting nested lists inside its items as well. + * + * This is needed because `tr.setNodeMarkup` can only swap a node's own type; + * when the list families differ, the item nodes themselves are incompatible + * node types (taskItem vs listItem have different specs and attributes), so + * the whole subtree has to be recreated node by node. + * + * Returns `null` when the rebuilt content would not fit the target content + * models, so callers can leave the original node untouched instead of + * producing an invalid document. Family-specific item attributes with no + * equivalent on the target item type (e.g. taskItem's `checked`) are + * intentionally dropped — a bullet list has no checked state to carry them. + */ +function rebuildListNode( + node: ProseMirrorNode, + listType: NodeType, + itemType: NodeType, + extensions: Extensions, +): ProseMirrorNode | null { + const newItems: ProseMirrorNode[] = [] + + for (let itemIndex = 0; itemIndex < node.childCount; itemIndex += 1) { + const item = node.child(itemIndex) + const newChildren: ProseMirrorNode[] = [] + + for (let childIndex = 0; childIndex < item.childCount; childIndex += 1) { + const child = item.child(childIndex) + + if (isList(child.type.name, extensions) && child.type !== listType) { + newChildren.push(rebuildListNode(child, listType, itemType, extensions) ?? child) + } else { + newChildren.push(child) + } + } + + const itemContent = Fragment.from(newChildren) + + if (!itemType.validContent(itemContent)) { + return null + } + + newItems.push( + itemType.create(item.type === itemType ? item.attrs : null, itemContent, item.marks), + ) + } + + const listContent = Fragment.from(newItems) + + if (!listType.validContent(listContent)) { + return null + } + + return listType.create(null, listContent) +} + /** * `tr.setNodeMarkup` only changes the type of the single node at the given - * position. When a list contains nested sublists (e.g. a bulletList whose - * list items contain further bulletList nodes), only the outermost list node - * was being converted, leaving nested sublists on their original type and - * breaking the visual hierarchy. This walks the whole list subtree and - * converts every nested list node that still has the original type. + * position. When a list contains nested sublists, only the outermost list + * node was being converted, leaving nested sublists on their original type + * and breaking the visual hierarchy. This walks the whole list subtree and + * converts every nested list node that is not already `listType`: + * + * - same item family (bulletList <-> orderedList): the existing items are + * already valid inside the target list, so only the list wrapper's type + * is swapped via `setNodeMarkup`, and the walk keeps descending. + * - different item family (taskList <-> bulletList/orderedList): the item + * node types are incompatible, so the whole subtree is rebuilt in one go + * via `rebuildListNode` and replaced; the walk skips its interior since + * the rebuild already handled any deeper nesting. + * + * Every conversion here is size-preserving (setNodeMarkup never changes + * sizes; the rebuild keeps the exact node structure and only swaps wrapper + * types), so all positions collected against the unmutated doc stay valid + * for the whole pass. */ function convertNestedLists( tr: Transaction, listPos: number, - oldListType: NodeType, - newListType: NodeType, + listType: NodeType, + itemType: NodeType, + extensions: Extensions, ): void { const listNode = tr.doc.nodeAt(listPos) @@ -78,23 +150,69 @@ function convertNestedLists( return } - const positions: number[] = [] + const sameFamilyPositions: number[] = [] + const crossFamilyNodes: { pos: number; node: ProseMirrorNode }[] = [] listNode.descendants((node, relativePos) => { - if (node.type === oldListType) { - positions.push(listPos + 1 + relativePos) + if (!isList(node.type.name, extensions) || node.type === listType) { + return true } - return true + + const pos = listPos + 1 + relativePos + + if (listType.validContent(node.content)) { + sameFamilyPositions.push(pos) + return true + } + + crossFamilyNodes.push({ pos, node }) + return false }) - // Convert deepest nodes first so earlier position lookups in this pass - // (captured before any mutation) remain valid — setNodeMarkup does not - // change node sizes, only its type/attrs, so positions never shift. - for (const pos of positions) { - tr.setNodeMarkup(pos, newListType) + for (const pos of sameFamilyPositions) { + tr.setNodeMarkup(pos, listType) + } + + for (const { pos, node } of crossFamilyNodes) { + const rebuilt = rebuildListNode(node, listType, itemType, extensions) + + if (rebuilt) { + tr.replaceWith(pos, pos + node.nodeSize, rebuilt) + } } } +/** + * Convert nested lists inside every `listType` list overlapping the current + * selection. Used after the `wrapInList` fallback path (taken for + * cross-family toggles like bulletList -> taskList, where the target list + * cannot hold the current items directly): `wrapInList` + `clearNodes` + * produce the correct top-level list, but any sublist nested inside the + * original items survives untouched on its old type. + */ +function convertNestedListsInSelection( + tr: Transaction, + listType: NodeType, + itemType: NodeType, + extensions: Extensions, +): void { + const { from, to } = tr.selection + const listPositions: number[] = [] + + tr.doc.nodesBetween(from, to, (node, pos) => { + if (node.type === listType) { + listPositions.push(pos) + return false + } + + return true + }) + + // All conversions are size-preserving, so positions collected up front + // remain valid across the whole loop. + listPositions.forEach(pos => convertNestedLists(tr, pos, listType, itemType, extensions)) +} + const joinListForwards = (tr: Transaction, listType: NodeType): boolean => { const list = findParentNode(node => node.type === listType)(tr.selection) @@ -242,12 +360,10 @@ export const toggleList: RawCommands['toggleList'] = isList(currentList.node.type.name, extensions) && listType.validContent(currentList.node.content) ) { - const oldListType = currentList.node.type - return chain() .command(() => { tr.setNodeMarkup(currentList.pos, listType) - convertNestedLists(tr, currentList.pos, oldListType, listType) + convertNestedLists(tr, currentList.pos, listType, itemType, extensions) return true }) @@ -269,6 +385,11 @@ export const toggleList: RawCommands['toggleList'] = return commands.clearNodes() }) .wrapInList(listType, attributes) + .command(() => { + convertNestedListsInSelection(tr, listType, itemType, extensions) + + return true + }) .command(() => joinListBackwards(tr, listType)) .command(() => joinListForwards(tr, listType)) .run() @@ -291,6 +412,11 @@ export const toggleList: RawCommands['toggleList'] = return commands.clearNodes() }) .wrapInList(listType, attributes) + .command(() => { + convertNestedListsInSelection(tr, listType, itemType, extensions) + + return true + }) .command(() => joinListBackwards(tr, listType)) .command(() => joinListForwards(tr, listType)) .run() diff --git a/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts b/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts index c88fdeb164..4f772dbf79 100644 --- a/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts +++ b/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts @@ -1,3 +1,4 @@ +import type { JSONContent } from '@tiptap/core' import { Editor } from '@tiptap/core' import Document from '@tiptap/extension-document' import Paragraph from '@tiptap/extension-paragraph' @@ -9,15 +10,21 @@ import { BulletList, ListItem, OrderedList, TaskItem, TaskList } from '../src/in // taskList has a different content model (taskItem+) than bulletList/orderedList // (listItem+), so cross-family conversions go through toggleList's wrapInList // + clearNodes fallback branch rather than the setNodeMarkup "change list -// type" branch used for bulletList <-> orderedList. That fallback had a -// separate, more severe bug: clearNodes() lifted every non-text node in the -// selection (including list/listItem/taskList/taskItem wrapper nodes, not -// just textblocks) in document order, using positions mapped against a doc -// that earlier iterations in the same pass had already mutated. For a -// taskList nested inside a taskItem inside a taskList, this ejected the -// nested item's paragraph out of the list entirely, orphaning it as a bare -// top-level paragraph sibling — outright content loss, not just a wrong -// list type. +// type" branch used for bulletList <-> orderedList. That fallback had two +// bugs: +// +// 1. clearNodes() lifted every non-text node in the selection (including +// list/listItem/taskList/taskItem wrapper nodes, not just textblocks) in +// document order, using positions mapped against a doc that earlier +// iterations in the same pass had already mutated. For a taskList nested +// inside a taskItem inside a taskList, this ejected the nested item's +// paragraph out of the list entirely — outright content loss. +// +// 2. Nested sublists that survived inside the wrapped items kept their old +// family entirely (taskList stayed taskList inside the new bulletList, +// and vice versa), because setNodeMarkup can't convert across families: +// the item node types themselves (taskItem vs listItem) are incompatible +// and the subtree has to be rebuilt node by node. describe('toggleList with taskList (cross-family, nested content)', () => { let editor: Editor @@ -42,6 +49,24 @@ describe('toggleList with taskList (cross-family, nested content)', () => { return editor } + const collectNodeTypes = (node: JSONContent, wanted: string[], acc: string[] = []): string[] => { + if (node.type && wanted.includes(node.type)) { + acc.push(node.type) + } + ;(node.content || []).forEach(child => collectNodeTypes(child, wanted, acc)) + return acc + } + + const listTypesIn = (json: JSONContent) => + collectNodeTypes({ type: 'doc', content: json.content }, [ + 'bulletList', + 'orderedList', + 'taskList', + ]) + + const itemTypesIn = (json: JSONContent) => + collectNodeTypes({ type: 'doc', content: json.content }, ['listItem', 'taskItem']) + const nestedTaskListDoc = `
  • @@ -64,24 +89,27 @@ describe('toggleList with taskList (cross-family, nested content)', () => {
` - it('does not drop nested content when converting nested taskList to bulletList', () => { + it('fully converts a nested taskList to bulletList, including the nested sublist', () => { setup(nestedTaskListDoc) editor.commands.selectAll() editor.commands.toggleBulletList() - // The critical invariant: no text is lost. Previously "Item 1-1" was - // ejected out of the list as an orphaned top-level paragraph. + // No text is lost. Previously "Item 1-1" was ejected out of the list + // as an orphaned top-level paragraph. expect(editor.getText()).toContain('Item 1') expect(editor.getText()).toContain('Item 1-1') const json = editor.getJSON() - // Everything stays inside a single top-level list — nothing orphaned - // as a bare sibling paragraph outside of it. + // Everything stays inside a single top-level list — nothing orphaned. expect(json.content).toHaveLength(1) expect(json.content![0].type).toBe('bulletList') + + // The nested sublist converts too: no taskList/taskItem remains anywhere. + expect(listTypesIn(json)).toEqual(['bulletList', 'bulletList']) + expect(itemTypesIn(json)).toEqual(['listItem', 'listItem']) }) - it('does not drop nested content when converting nested bulletList to taskList', () => { + it('fully converts a nested bulletList to taskList, including the nested sublist', () => { setup(nestedBulletListDoc) editor.commands.selectAll() editor.commands.toggleTaskList() @@ -92,9 +120,16 @@ describe('toggleList with taskList (cross-family, nested content)', () => { const json = editor.getJSON() expect(json.content).toHaveLength(1) expect(json.content![0].type).toBe('taskList') + + expect(listTypesIn(json)).toEqual(['taskList', 'taskList']) + expect(itemTypesIn(json)).toEqual(['taskItem', 'taskItem']) + + // Rebuilt task items get the taskItem defaults (unchecked). + const outerItem = json.content![0].content![0] + expect(outerItem.attrs?.checked).toBe(false) }) - it('does not drop nested content when converting nested taskList to orderedList', () => { + it('fully converts a nested taskList to orderedList, including the nested sublist', () => { setup(nestedTaskListDoc) editor.commands.selectAll() editor.commands.toggleOrderedList() @@ -105,5 +140,81 @@ describe('toggleList with taskList (cross-family, nested content)', () => { const json = editor.getJSON() expect(json.content).toHaveLength(1) expect(json.content![0].type).toBe('orderedList') + + expect(listTypesIn(json)).toEqual(['orderedList', 'orderedList']) + expect(itemTypesIn(json)).toEqual(['listItem', 'listItem']) + }) + + it('converts a cross-family sublist nested inside a same-family toggle', () => { + // Top-level bulletList -> orderedList is a same-family conversion + // (setNodeMarkup branch), but the taskList nested inside one of its + // items still needs the cross-family rebuild. + setup(` +
    +
  • +

    Item 1

    +
      +

    • Item 1-1

      • +
    +
    • +
+ `) + editor.commands.selectAll() + editor.commands.toggleOrderedList() + + expect(editor.getText()).toContain('Item 1') + expect(editor.getText()).toContain('Item 1-1') + + const json = editor.getJSON() + expect(json.content).toHaveLength(1) + expect(listTypesIn(json)).toEqual(['orderedList', 'orderedList']) + expect(itemTypesIn(json)).toEqual(['listItem', 'listItem']) + }) + + it('drops the checked state when leaving the task family and round-trips without crashing', () => { + // Bullet lists have no checked state, so converting away from taskList + // intentionally drops it; converting back yields taskItem defaults. + setup(` +
    +

  • Done thing

    • +
+ `) + editor.commands.selectAll() + editor.commands.toggleBulletList() + + editor.commands.selectAll() + editor.commands.toggleTaskList() + + const json = editor.getJSON() + expect(editor.getText()).toContain('Done thing') + expect(json.content![0].type).toBe('taskList') + expect(json.content![0].content![0].attrs?.checked).toBe(false) + }) + + it('converts a 3-level deep mixed nesting entirely', () => { + setup(` +
    +
  • +

    A

    +
      +
    • +

      A-1

      +
        +

      • A-1-a

        • +
      +
      • +
    +
    • +
+ `) + editor.commands.selectAll() + editor.commands.toggleBulletList() + + const json = editor.getJSON() + expect(editor.getText()).toContain('A') + expect(editor.getText()).toContain('A-1') + expect(editor.getText()).toContain('A-1-a') + expect(listTypesIn(json)).toEqual(['bulletList', 'bulletList', 'bulletList']) + expect(itemTypesIn(json)).toEqual(['listItem', 'listItem', 'listItem']) }) })