fix(extension-list): improve list item node view performance (remirro…

…r#1242)
sandzone · Sep 17, 2021 · 1c31320 · 1c31320
1 parent 8800d3c
commit 1c31320
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 58 deletions.
diff --git a/.changeset/unlucky-birds-hug.md b/.changeset/unlucky-birds-hug.md
@@ -0,0 +1,5 @@
+---
+'@remirror/extension-list': patch
+---
+
+Improve the performance of large task lists and collapsible bullet lists.
diff --git a/packages/remirror__extension-list/src/list-item-extension.ts b/packages/remirror__extension-list/src/list-item-extension.ts
@@ -13,6 +13,7 @@ import {
   NodeSpecOverride,
   NodeViewMethod,
   ProsemirrorAttributes,
+  ProsemirrorNode,
   Static,
 } from '@remirror/core';
 import { NodeSelection } from '@remirror/pm/state';
@@ -73,25 +74,23 @@ export class ListItemExtension extends NodeExtension<ListItemOptions> {
       const mark: HTMLElement = document.createElement('div');
       mark.classList.add(ExtensionListTheme.COLLAPSIBLE_LIST_ITEM_BUTTON);
       mark.contentEditable = 'false';
-
-      if (node.childCount <= 1) {
-        mark.classList.add('disabled');
-      } else {
-        mark.addEventListener('click', () => {
-          const pos = (getPos as () => number)();
-          const selection = NodeSelection.create(view.state.doc, pos);
-          view.dispatch(view.state.tr.setSelection(selection));
-          this.store.commands.toggleListItemClosed();
-          return true;
-        });
-      }
+      mark.addEventListener('click', () => {
+        if (mark.classList.contains('disabled')) {
+          return;
+        }
+
+        const pos = (getPos as () => number)();
+        const selection = NodeSelection.create(view.state.doc, pos);
+        view.dispatch(view.state.tr.setSelection(selection));
+        this.store.commands.toggleListItemClosed();
+        return true;
+      });
 
       return createCustomMarkListItemNodeView({
-        view: this.store.view,
         mark,
-        extraClasses: node.attrs.closed
-          ? [ExtensionListTheme.COLLAPSIBLE_LIST_ITEM_CLOSED]
-          : undefined,
+        node,
+        updateDOM: updateNodeViewDOM,
+        updateMark: updateNodeViewMark,
       });
     };
   }
@@ -138,6 +137,16 @@ export class ListItemExtension extends NodeExtension<ListItemOptions> {
   }
 }
 
+function updateNodeViewDOM(node: ProsemirrorNode, dom: HTMLElement) {
+  node.attrs.closed
+    ? dom.classList.add(ExtensionListTheme.COLLAPSIBLE_LIST_ITEM_CLOSED)
+    : dom.classList.remove(ExtensionListTheme.COLLAPSIBLE_LIST_ITEM_CLOSED);
+}
+
+function updateNodeViewMark(node: ProsemirrorNode, mark: HTMLElement) {
+  node.childCount <= 1 ? mark.classList.add('disabled') : mark.classList.remove('disabled');
+}
+
 export interface ListItemOptions {
   /**
    * Set this to true to support toggling.

diff --git a/packages/remirror__extension-list/src/list-item-node-view.ts b/packages/remirror__extension-list/src/list-item-node-view.ts
@@ -1,52 +1,44 @@
-import type { EditorView, NodeView } from '@remirror/pm/view';
+import { ProsemirrorNode } from '@remirror/pm';
+import type { NodeView } from '@remirror/pm/view';
 import { ExtensionListTheme } from '@remirror/theme';
 
+type UpdateElement = (node: ProsemirrorNode, dom: HTMLElement) => void;
+
 export function createCustomMarkListItemNodeView({
+  node,
   mark,
-  extraAttrs,
-  extraClasses,
-  view,
+  updateDOM,
+  updateMark,
 }: {
+  node: ProsemirrorNode;
   mark: HTMLElement;
-  extraAttrs?: Record<string, string>;
-  extraClasses?: string[];
-  view: EditorView;
+  updateDOM: UpdateElement;
+  updateMark: UpdateElement;
 }): NodeView {
-  const dom = document.createElement('li');
-  dom.classList.add(ExtensionListTheme.LIST_ITEM_WITH_CUSTOM_MARKER);
-
-  if (extraClasses) {
-    extraClasses.forEach((className) => dom.classList.add(className));
-  }
-
   const markContainer = document.createElement('span');
   markContainer.contentEditable = 'false';
   markContainer.classList.add(ExtensionListTheme.LIST_ITEM_MARKER_CONTAINER);
+  markContainer.append(mark);
 
   const contentDOM = document.createElement('div');
 
-  markContainer.append(mark);
+  const dom = document.createElement('li');
+  dom.classList.add(ExtensionListTheme.LIST_ITEM_WITH_CUSTOM_MARKER);
   dom.append(markContainer);
   dom.append(contentDOM);
 
-  // When a list item node's `content` updates, it's necessary to re-run the
-  // nodeView function so that the list item node's `disabled` class can be
-  // updated.
-  //
-  // However, when users are using IME, never re-create the nodeView. See also #1017.
-  const update = (): boolean => {
-    if (view?.composing) {
-      return true;
+  const update = (newNode: ProsemirrorNode): boolean => {
+    if (newNode.type !== node.type) {
+      return false;
     }
 
-    return false;
+    node = newNode;
+    updateDOM(node, dom);
+    updateMark(node, mark);
+    return true;
   };
 
-  if (extraAttrs) {
-    for (const [key, value] of Object.entries(extraAttrs)) {
-      dom.setAttribute(key, value);
-    }
-  }
+  update(node);
 
   return { dom, contentDOM, update };
 }
diff --git a/packages/remirror__extension-list/src/task-list-item-extension.ts b/packages/remirror__extension-list/src/task-list-item-extension.ts
@@ -14,6 +14,7 @@ import {
   NodeSpecOverride,
   NodeViewMethod,
   ProsemirrorAttributes,
+  ProsemirrorNode,
 } from '@remirror/core';
 import { InputRule } from '@remirror/pm/inputrules';
 import { ResolvedPos } from '@remirror/pm/model';
@@ -83,25 +84,22 @@ export class TaskListItemExtension extends NodeExtension {
 
   createNodeViews(): NodeViewMethod | Record<string, never> {
     return (node, view, getPos) => {
-      const checkbox = document.createElement('input');
-      checkbox.type = 'checkbox';
-      checkbox.checked = !!node.attrs.checked;
-      checkbox.classList.add(ExtensionListTheme.LIST_ITEM_CHECKBOX);
-      checkbox.contentEditable = 'false';
-      checkbox.addEventListener('click', () => {
+      const mark = document.createElement('input');
+      mark.type = 'checkbox';
+      mark.classList.add(ExtensionListTheme.LIST_ITEM_CHECKBOX);
+      mark.contentEditable = 'false';
+      mark.addEventListener('click', () => {
         const pos = (getPos as () => number)();
         const $pos = view.state.doc.resolve(pos + 1);
         this.store.commands.toggleCheckboxChecked({ $pos });
         return true;
       });
 
-      const extraAttrs: Record<string, string> = node.attrs.checked
-        ? { 'data-task-list-item': '', 'data-checked': '' }
-        : { 'data-task-list-item': '' };
       return createCustomMarkListItemNodeView({
-        view: this.store.view,
-        mark: checkbox,
-        extraAttrs,
+        node,
+        mark,
+        updateDOM: updateNodeViewDOM,
+        updateMark: updateNodeViewMark,
       });
     };
   }
@@ -216,6 +214,15 @@ export class TaskListItemExtension extends NodeExtension {
   }
 }
 
+function updateNodeViewDOM(node: ProsemirrorNode, dom: HTMLElement) {
+  node.attrs.checked ? dom.setAttribute('data-checked', '') : dom.removeAttribute('data-checked');
+  dom.setAttribute('data-task-list-item', '');
+}
+
+function updateNodeViewMark(node: ProsemirrorNode, mark: HTMLElement) {
+  (mark as HTMLInputElement).checked = !!node.attrs.checked;
+}
+
 export type TaskListItemAttributes = ProsemirrorAttributes<{
   /**
    * @default false

diff --git a/support/root/.eslintrc.js b/support/root/.eslintrc.js
@@ -83,7 +83,6 @@ let config = {
     'unicorn/prefer-date-now': 'error',
     'unicorn/prefer-default-parameters': 'error',
     'unicorn/prefer-dom-node-append': 'error',
-    'unicorn/prefer-dom-node-dataset': 'error',
     'unicorn/prefer-dom-node-remove': 'error',
     'unicorn/prefer-dom-node-text-content': 'error',
     'unicorn/prefer-includes': 'error',