Merge pull request #454 from prezly/fix/care-1965-error-could-not-com…

…pletely-normalize-the-editor-after-4158-iterations

[CARE-1965] Fix - Infinite list structure normalization loop

package.json

@@ -72,6 +72,7 @@
         "babel-plugin-module-resolver": "^5.0.0",
         "babel-plugin-transform-remove-imports": "^1.7.0",
         "branch-pipe": "^1.0.1",
+        "canvas": "^2.11.2",
         "concurrently": "^6.4.0",
         "eslint": "^8.18.0",
         "eslint-config-prettier": "^8.5.0",

packages/slate-editor/src/modules/editor/Editor.test.tsx

@@ -321,6 +321,30 @@ describe('Editor', () => {
             expect(editor.children).toMatchObject(JSON.parse(expected));
         });
 
+        /**
+         * @see CARE-1965
+         */
+        it('should normalize list-items directly nested into another list-item', () => {
+            const editor = createEditor(
+                <editor>
+                    <h-p>
+                        <h-text>
+                            <cursor />
+                        </h-text>
+                    </h-p>
+                </editor>,
+            );
+
+            const input = readTestFile('input/list-normalization-4.json');
+            const expected = readTestFile('expected/list-normalization-4.json');
+
+            editor.children = JSON.parse(input).children;
+
+            Editor.normalize(editor, { force: true });
+
+            expect(editor.children).toMatchObject(JSON.parse(expected));
+        });
+
         /**
          * @see CARE-1320
          */

packages/slate-editor/src/modules/editor/__tests__/expected/list-normalization-4.json

@@ -0,0 +1,18 @@
+[
+    {
+        "type": "paragraph",
+        "children": [
+            {
+                "text": "Hello"
+            }
+        ]
+    },
+    {
+        "type": "paragraph",
+        "children": [
+            {
+                "text": ""
+            }
+        ]
+    }
+]

packages/slate-editor/src/modules/editor/__tests__/input/list-normalization-4.json

@@ -0,0 +1,32 @@
+{
+    "type": "document",
+    "children": [
+        {
+            "type": "list-item",
+            "children": [
+                {
+                    "type": "list-item-text",
+                    "children": [
+                        {
+                            "text": "Hello"
+                        }
+                    ]
+                },
+                {
+                    "type": "list-item",
+                    "children": [
+                        {
+                            "type": "list-item-text",
+                            "children": [
+                                {
+                                    "text": ""
+                                }
+                            ]
+                        }
+                    ]
+                }
+            ]
+        }
+    ],
+    "version": "0.50"
+}

packages/slate-editor/src/modules/editor/test-utils.ts

@@ -4,6 +4,7 @@ import type { Editor } from 'slate';
 
 import type { EditorEventMap } from '#modules/events';
 import { withEvents } from '#modules/events';
+import { hierarchySchema, withNodesHierarchy } from '#modules/nodes-hierarchy';
 import { coverage, createDelayedResolve, oembedInfo } from '#modules/tests';
 
 import { createEditor as createBaseEditor } from './createEditor';
@@ -102,5 +103,8 @@ export function getAllExtensions() {
 }
 
 export function createEditor(input: JSX.Element) {
-    return createBaseEditor(input as unknown as Editor, getAllExtensions, [withEvents(events)]);
+    return createBaseEditor(input as unknown as Editor, getAllExtensions, [
+        withEvents(events),
+        withNodesHierarchy(hierarchySchema),
+    ]);
 }

packages/slate-lists/src/lib/index.ts

@@ -10,6 +10,8 @@ export { getParentListItem } from './getParentListItem';
 export { getPrevSibling } from './getPrevSibling';
 export { isAtStartOfListItem } from './isAtStartOfListItem';
 export { isAtEmptyListItem } from './isAtEmptyListItem';
+export { isContainingTextNodes } from './isContainingTextNodes';
+export { isElementOrEditor } from './isElementOrEditor';
 export { isInList } from './isInList';
 export { isListItemContainingText } from './isListItemContainingText';
 export { pickSubtreesRoots } from './pickSubtreesRoots';

packages/slate-lists/src/lib/isContainingTextNodes.ts

@@ -0,0 +1,6 @@
+import type { Element } from 'slate';
+import { Text } from 'slate';
+
+export function isContainingTextNodes(element: Element): boolean {
+    return element.children.some(Text.isText);
+}

packages/slate-lists/src/lib/isElementOrEditor.ts

@@ -0,0 +1,8 @@
+import type { Editor, Element, Node } from 'slate';
+
+/**
+ * The Slate's `Element.isElement()` is explicitly excluding `Editor`.
+ */
+export function isElementOrEditor(node: Node): node is Element | Editor {
+    return 'children' in node;
+}

packages/slate-lists/src/normalizations/normalizeListChildren.ts

@@ -1,5 +1,5 @@
 import type { Editor, NodeEntry } from 'slate';
-import { Element, Node, Text, Transforms } from 'slate';
+import { Node, Text, Transforms } from 'slate';
 
 import type { ListsSchema } from '../types';
 
@@ -18,31 +18,23 @@ export function normalizeListChildren(
         return false;
     }
 
-    let normalized = false;
-
     const children = Array.from(Node.children(editor, path));
 
-    children.forEach(([childNode, childPath]) => {
-        if (normalized) {
-            // Make sure at most 1 normalization operation is done at a time.
-            return;
-        }
-
+    for (const [childNode, childPath] of children) {
         if (Text.isText(childNode)) {
             // This can happen during pasting
 
             // When pasting from MS Word there may be weird text nodes with some whitespace
             // characters. They're not expected to be deserialized so we remove them.
-            if (!childNode.text.trim()) {
+            if (childNode.text.trim() === '') {
                 if (children.length > 1) {
                     Transforms.removeNodes(editor, { at: childPath });
                 } else {
                     // If we're removing the only child, we may delete the whole list as well
                     // to avoid never-ending normalization (Slate will insert empty text node).
                     Transforms.removeNodes(editor, { at: path });
                 }
-                normalized = true;
-                return;
+                return true;
             }
 
             Transforms.wrapNodes(
@@ -52,33 +44,26 @@ export function normalizeListChildren(
                 }),
                 { at: childPath },
             );
-            normalized = true;
-            return;
-        }
-
-        if (!Element.isElement(childNode)) {
-            return;
+            return true;
         }
 
         if (schema.isListItemTextNode(childNode)) {
             Transforms.wrapNodes(editor, schema.createListItemNode(), { at: childPath });
-            normalized = true;
-            return;
+            return true;
         }
 
         if (schema.isListNode(childNode)) {
             // Wrap it into a list item so that `normalizeOrphanNestedList` can take care of it.
             Transforms.wrapNodes(editor, schema.createListItemNode(), { at: childPath });
-            normalized = true;
-            return;
+            return true;
         }
 
         if (!schema.isListItemNode(childNode)) {
             Transforms.setNodes(editor, schema.createListItemTextNode(), { at: childPath });
             Transforms.wrapNodes(editor, schema.createListItemNode(), { at: childPath });
-            normalized = true;
+            return true;
         }
-    });
+    }
 
-    return normalized;
+    return false;
 }

packages/slate-lists/src/normalizations/normalizeListItemChildren.ts

@@ -18,9 +18,7 @@ export function normalizeListItemChildren(
 
     const children = Array.from(Node.children(editor, path));
 
-    for (let childIndex = 0; childIndex < children.length; ++childIndex) {
-        const [childNode, childPath] = children[childIndex];
-
+    for (const [childIndex, [childNode, childPath]] of children.entries()) {
         if (Text.isText(childNode) || editor.isInline(childNode)) {
             const listItemText = schema.createListItemTextNode({
                 children: [childNode],

packages/slate-lists/src/normalizations/normalizeOrphanListItem.ts

@@ -1,7 +1,7 @@
-import type { Node, NodeEntry } from 'slate';
-import { Editor, Transforms } from 'slate';
+import type { Node, NodeEntry, Editor } from 'slate';
+import { Element, Transforms } from 'slate';
 
-import { getParentList } from '../lib';
+import { isContainingTextNodes, isElementOrEditor } from '../lib';
 import type { ListsSchema } from '../types';
 
 /**
@@ -17,22 +17,24 @@ export function normalizeOrphanListItem(
     schema: ListsSchema,
     [node, path]: NodeEntry<Node>,
 ): boolean {
-    if (!schema.isListItemNode(node)) {
-        // This function does not know how to normalize other nodes.
-        return false;
+    if (isElementOrEditor(node) && !schema.isListNode(node)) {
+        // We look for "list-item" nodes that are NOT under a "list" node
+        for (const [index, child] of node.children.entries()) {
+            if (Element.isElement(child) && schema.isListItemNode(child)) {
+                if (isContainingTextNodes(child)) {
+                    Transforms.setNodes(editor, schema.createDefaultTextNode(), {
+                        at: [...path, index],
+                    });
+                } else {
+                    Transforms.unwrapNodes(editor, {
+                        at: [...path, index],
+                        mode: 'highest',
+                    });
+                }
+                return true;
+            }
+        }
     }
 
-    const parentList = getParentList(editor, schema, path);
-
-    if (parentList) {
-        // If there is a parent "list", then the fix does not apply.
-        return false;
-    }
-
-    Editor.withoutNormalizing(editor, () => {
-        Transforms.unwrapNodes(editor, { at: path });
-        Transforms.setNodes(editor, schema.createDefaultTextNode(), { at: path });
-    });
-
-    return true;
+    return false;
 }

packages/slate-lists/src/normalizations/normalizeOrphanListItemText.ts

@@ -1,7 +1,7 @@
 import type { Editor, Node, NodeEntry } from 'slate';
-import { Transforms } from 'slate';
+import { Element, Transforms } from 'slate';
 
-import { getParentListItem } from '../lib';
+import { isContainingTextNodes, isElementOrEditor } from '../lib';
 import type { ListsSchema } from '../types';
 
 /**
@@ -17,19 +17,24 @@ export function normalizeOrphanListItemText(
     schema: ListsSchema,
     [node, path]: NodeEntry<Node>,
 ): boolean {
-    if (!schema.isListItemTextNode(node)) {
-        // This function does not know how to normalize other nodes.
-        return false;
+    if (isElementOrEditor(node) && !schema.isListItemNode(node)) {
+        // We look for "list-item-text" nodes that are NOT under a "list-item" node
+        for (const [index, child] of node.children.entries()) {
+            if (Element.isElement(child) && schema.isListItemTextNode(child)) {
+                if (isContainingTextNodes(child)) {
+                    Transforms.setNodes(editor, schema.createDefaultTextNode(), {
+                        at: [...path, index],
+                    });
+                } else {
+                    Transforms.unwrapNodes(editor, {
+                        at: [...path, index],
+                        mode: 'highest',
+                    });
+                }
+                return true;
+            }
+        }
     }
 
-    const parentListItem = getParentListItem(editor, schema, path);
-
-    if (parentListItem) {
-        // If there is a parent "list-item", then the fix does not apply.
-        return false;
-    }
-
-    Transforms.setNodes(editor, schema.createDefaultTextNode(), { at: path });
-
-    return true;
+    return false;
 }

packages/slate-lists/src/normalizations/normalizeSiblingLists.ts

@@ -22,9 +22,9 @@ export function normalizeSiblingLists(
     const [, path] = entry;
     const nextSibling = getNextSibling(editor, path);
 
-    if (!nextSibling) {
-        return false;
+    if (nextSibling) {
+        return mergeListWithPreviousSiblingList(editor, schema, nextSibling);
     }
 
-    return mergeListWithPreviousSiblingList(editor, schema, nextSibling);
+    return false;
 }