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/.changeset/fix-toggle-list-nested-structure.md b/.changeset/fix-toggle-list-nested-structure.md new file mode 100644 index 0000000000..4f4561e67c --- /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 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/__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/core/src/commands/toggleList.ts b/packages/core/src/commands/toggleList.ts index d040d0c266..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,6 +59,160 @@ 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, 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, + listType: NodeType, + itemType: NodeType, + extensions: Extensions, +): void { + const listNode = tr.doc.nodeAt(listPos) + + if (!listNode) { + return + } + + const sameFamilyPositions: number[] = [] + const crossFamilyNodes: { pos: number; node: ProseMirrorNode }[] = [] + + listNode.descendants((node, relativePos) => { + if (!isList(node.type.name, extensions) || node.type === listType) { + return true + } + + const pos = listPos + 1 + relativePos + + if (listType.validContent(node.content)) { + sameFamilyPositions.push(pos) + return true + } + + crossFamilyNodes.push({ pos, node }) + return false + }) + + 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) @@ -208,6 +363,7 @@ export const toggleList: RawCommands['toggleList'] = return chain() .command(() => { tr.setNodeMarkup(currentList.pos, listType) + convertNestedLists(tr, currentList.pos, listType, itemType, extensions) return true }) @@ -229,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() @@ -251,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__/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) + }) +}) diff --git a/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts b/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts new file mode 100644 index 0000000000..4f772dbf79 --- /dev/null +++ b/packages/extension-list/__tests__/toggleListTaskListCrossFamily.spec.ts @@ -0,0 +1,220 @@ +import type { JSONContent } from '@tiptap/core' +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 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 + + 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 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 = ` + + ` + + const nestedBulletListDoc = ` + + ` + + it('fully converts a nested taskList to bulletList, including the nested sublist', () => { + setup(nestedTaskListDoc) + editor.commands.selectAll() + editor.commands.toggleBulletList() + + // 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. + 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('fully converts a nested bulletList to taskList, including the nested sublist', () => { + 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') + + 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('fully converts a nested taskList to orderedList, including the nested sublist', () => { + 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') + + 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(` + + `) + 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(` + + `) + 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(` + + `) + 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']) + }) +})