Improve TypeScript types by removing [k: string]: any from LexicalNode

This refactoring improves the ability for TypeScript to catch errors, particularly typos, because otherwise any unknown method would pass the type checker. I did find one bug in LinkNode (facebook#5221) and some tests that were using textNode.length instead of textNode.__text.length. The most awkward part was adding an explicit type to the constructors for the classes to implement Klass<T> more precisely. I think that subclasses shouldn't typically need to do this since the superclass' type will generally be sufficient. Another perhaps controversial addition was adding branded types for isNestedListNode and $isRootOrShadowRoot so that a more precise type could be refined with a predicate in a way that doesn't make TypeScript try and infer the wrong thing (e.g. if `f(x): x is Node` returns false then it will also infer `!(x is Node)` but that is not desired when we are looking for something more precise than the type alone.
etrepum · Dec 6, 2023 · 1126277 · 1126277
1 parent 0f2e1fd
commit 1126277
Show file tree

Hide file tree

Showing 38 changed files with 287 additions and 125 deletions.
diff --git a/packages/lexical-clipboard/src/clipboard.ts b/packages/lexical-clipboard/src/clipboard.ts
@@ -38,6 +38,7 @@ import {
   LexicalNode,
   RangeSelection,
   SELECTION_CHANGE_COMMAND,
+  SerializedElementNode,
   SerializedTextNode,
 } from 'lexical';
 import {CAN_USE_DOM} from 'shared/canUseDOM';
@@ -357,7 +358,6 @@ function exportNodeToJSON<T extends LexicalNode>(node: T): BaseSerializedNode {
   const serializedNode = node.exportJSON();
   const nodeClass = node.constructor;
 
-  // @ts-expect-error TODO Replace Class utility type with InstanceType
   if (serializedNode.type !== nodeClass.getType()) {
     invariant(
       false,
@@ -366,10 +366,9 @@ function exportNodeToJSON<T extends LexicalNode>(node: T): BaseSerializedNode {
     );
   }
 
-  // @ts-expect-error TODO Replace Class utility type with InstanceType
-  const serializedChildren = serializedNode.children;
-
   if ($isElementNode(node)) {
+    const serializedChildren = (serializedNode as SerializedElementNode)
+      .children;
     if (!Array.isArray(serializedChildren)) {
       invariant(
         false,

diff --git a/packages/lexical-code/src/__tests__/unit/LexicalCodeNode.test.ts b/packages/lexical-code/src/__tests__/unit/LexicalCodeNode.test.ts
@@ -21,9 +21,11 @@ import {
   $getNodeByKey,
   $getRoot,
   $getSelection,
+  $isElementNode,
   $isLineBreakNode,
   $isRangeSelection,
   $isTabNode,
+  $isTextNode,
   $setSelection,
   KEY_ARROW_DOWN_COMMAND,
   KEY_ARROW_UP_COMMAND,
@@ -217,6 +219,7 @@ describe('LexicalCodeNode tests', () => {
       // TODO consolidate editor.update - there's some bad logic in updateAndRetainSelection
       await editor.update(() => {
         const codeText = $getRoot().getFirstDescendant();
+        invariant($isTextNode(codeText));
         codeText.select(1, 'function'.length);
       });
       await editor.dispatchCommand(KEY_TAB_COMMAND, tabKeyboardEvent());
@@ -240,6 +243,7 @@ describe('LexicalCodeNode tests', () => {
       // TODO consolidate editor.update - there's some bad logic in updateAndRetainSelection
       await editor.update(() => {
         const codeText = $getRoot().getFirstDescendant();
+        invariant($isTextNode(codeText));
         codeText.select(0, 'function'.length);
       });
       await editor.dispatchCommand(KEY_TAB_COMMAND, tabKeyboardEvent());
@@ -276,6 +280,7 @@ describe('LexicalCodeNode tests', () => {
       // TODO consolidate editor.update - there's some bad logic in updateAndRetainSelection
       await editor.update(() => {
         const codeText = $getRoot().getFirstDescendant();
+        invariant($isTextNode(codeText));
         codeText.select(0, 0);
       });
       await editor.dispatchCommand(KEY_TAB_COMMAND, tabKeyboardEvent());
@@ -365,6 +370,7 @@ describe('LexicalCodeNode tests', () => {
       // TODO consolidate editor.update - there's some bad logic in updateAndRetainSelection
       await editor.update(() => {
         const codeText = $getRoot().getLastDescendant();
+        invariant($isTextNode(codeText));
         codeText.select(1, 1);
       });
       await editor.dispatchCommand(KEY_TAB_COMMAND, shiftTabKeyboardEvent());
@@ -482,6 +488,7 @@ describe('LexicalCodeNode tests', () => {
             'caret at start of line (first line)',
             () => {
               const code = $getRoot().getFirstChild();
+              invariant($isElementNode(code));
               code.selectStart();
             },
             () => {
@@ -559,11 +566,13 @@ describe('LexicalCodeNode tests', () => {
             'caret immediately before code (first line)',
             () => {
               const code = $getRoot().getFirstChild();
+              invariant($isElementNode(code));
+              const firstChild = code.getFirstChild();
+              invariant($isTextNode(firstChild));
               if (tabOrSpaces === 'tab') {
-                const firstTab = code.getFirstChild();
-                firstTab.getNextSibling().selectNext(0, 0);
+                firstChild.getNextSibling().selectNext(0, 0);
               } else {
-                code.getFirstChild().select(4, 4);
+                firstChild.select(4, 4);
               }
             },
             () => {
@@ -575,6 +584,7 @@ describe('LexicalCodeNode tests', () => {
               expect(selection.isCollapsed()).toBe(true);
               if (moveTo === 'start') {
                 const code = $getRoot().getFirstChild();
+                invariant($isElementNode(code));
                 const firstChild = code.getFirstChild();
                 expect(selection.anchor.getNode().is(firstChild)).toBe(true);
                 expect(selection.anchor.offset).toBe(0);
@@ -629,11 +639,13 @@ describe('LexicalCodeNode tests', () => {
             'caret in between space (first line)',
             () => {
               const code = $getRoot().getFirstChild();
+              invariant($isElementNode(code));
+              const firstChild = code.getFirstChild();
+              invariant($isTextNode(firstChild));
               if (tabOrSpaces === 'tab') {
-                const firstTab = code.getFirstChild();
-                firstTab.selectNext(0, 0);
+                firstChild.selectNext(0, 0);
               } else {
-                code.getFirstChild().select(2, 2);
+                firstChild.select(2, 2);
               }
             },
             () => {
@@ -720,6 +732,7 @@ describe('LexicalCodeNode tests', () => {
                 $isCodeHighlightNode(dfsNode.node),
               )[tabOrSpaces === 'tab' ? 0 : 1].node;
               const index = codeHighlight.getTextContent().indexOf('tion');
+              invariant($isTextNode(codeHighlight));
               codeHighlight.select(index, index);
             },
             () => {
@@ -761,6 +774,7 @@ describe('LexicalCodeNode tests', () => {
                 $isCodeHighlightNode(dfsNode.node),
               )[tabOrSpaces === 'tab' ? 1 : 2].node;
               const index = codeHighlight.getTextContent().indexOf('oo');
+              invariant($isTextNode(codeHighlight));
               codeHighlight.select(index, index);
             },
             () => {

diff --git a/packages/lexical-html/src/index.ts b/packages/lexical-html/src/index.ts
@@ -161,7 +161,7 @@ function getConversionFunction(
       if (
         domConversion !== null &&
         (currentConversion === null ||
-          currentConversion.priority < domConversion.priority)
+          (currentConversion.priority || 0) < (domConversion.priority || 0))
       ) {
         currentConversion = domConversion;
       }

diff --git a/packages/lexical-list/src/LexicalListItemNode.ts b/packages/lexical-list/src/LexicalListItemNode.ts
@@ -108,6 +108,7 @@ export class ListItemNode extends ElementNode {
       const parent = node.getParent();
       if ($isListNode(parent)) {
         updateChildrenListItemValue(parent);
+        invariant($isListItemNode(node), 'node is not a ListItemNode');
         if (parent.getListType() !== 'check' && node.getChecked() != null) {
           node.setChecked(undefined);
         }
@@ -194,6 +195,10 @@ export class ListItemNode extends ElementNode {
       replaceWithNode.insertAfter(newList);
     }
     if (includeChildren) {
+      invariant(
+        $isElementNode(replaceWithNode),
+        'includeChildren should only be true for ElementNodes',
+      );
       this.getChildren().forEach((child: LexicalNode) => {
         replaceWithNode.append(child);
       });

diff --git a/packages/lexical-list/src/utils.ts b/packages/lexical-list/src/utils.ts
@@ -6,7 +6,7 @@
  *
  */
 
-import type {LexicalNode} from 'lexical';
+import type {LexicalNode, Spread} from 'lexical';
 
 import invariant from 'shared/invariant';
 
@@ -124,14 +124,21 @@ export function $getAllListItems(node: ListNode): Array<ListItemNode> {
   return listItemNodes;
 }
 
+const NestedListNodeBrand: unique symbol = Symbol.for(
+  '@lexical/NestedListNodeBrand',
+);
+
 /**
  * Checks to see if the passed node is a ListItemNode and has a ListNode as a child.
  * @param node - The node to be checked.
  * @returns true if the node is a ListItemNode and has a ListNode child, false otherwise.
  */
 export function isNestedListNode(
   node: LexicalNode | null | undefined,
-): boolean {
+): node is Spread<
+  {getFirstChild(): ListNode; [NestedListNodeBrand]: never},
+  ListItemNode
+> {
   return $isListItemNode(node) && $isListNode(node.getFirstChild());
 }
 

diff --git a/packages/lexical-markdown/src/MarkdownImport.ts b/packages/lexical-markdown/src/MarkdownImport.ts
@@ -16,7 +16,7 @@ import type {
 import type {LexicalNode, TextNode} from 'lexical';
 
 import {$createCodeNode} from '@lexical/code';
-import {$isListItemNode, $isListNode} from '@lexical/list';
+import {$isListItemNode, $isListNode, ListItemNode} from '@lexical/list';
 import {$isQuoteNode} from '@lexical/rich-text';
 import {$findMatchingParent} from '@lexical/utils';
 import {
@@ -145,7 +145,7 @@ function importBlocks(
       $isQuoteNode(previousNode) ||
       $isListNode(previousNode)
     ) {
-      let targetNode: LexicalNode | null = previousNode;
+      let targetNode: typeof previousNode | ListItemNode | null = previousNode;
 
       if ($isListNode(previousNode)) {
         const lastDescendant = previousNode.getLastDescendant();

diff --git a/packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx b/packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx
@@ -30,6 +30,7 @@ import {
   HTMLTableElementWithWithTableSelectionState,
   TableCellHeaderStates,
   TableCellNode,
+  TableRowNode,
 } from '@lexical/table';
 import {
   $createParagraphNode,
@@ -431,7 +432,7 @@ function TableActionMenu({
       const tableColumnIndex =
         $getTableColumnIndexFromTableCellNode(tableCellNode);
 
-      const tableRows = tableNode.getChildren();
+      const tableRows = tableNode.getChildren<TableRowNode>();
       const maxRowsLength = Math.max(
         ...tableRows.map((row) => row.getChildren().length),
       );
@@ -485,7 +486,7 @@ function TableActionMenu({
 
             for (let i = 0; i < nodes.length; i++) {
               const node = nodes[i];
-              if (DEPRECATED_$isGridCellNode(node)) {
+              if ($isTableCellNode(node)) {
                 node.setBackgroundColor(value);
               }
             }

diff --git a/packages/lexical-playground/src/plugins/TableCellResizer/index.tsx b/packages/lexical-playground/src/plugins/TableCellResizer/index.tsx
@@ -19,6 +19,7 @@ import {
   $isTableCellNode,
   $isTableRowNode,
   getCellFromTarget,
+  TableCellNode,
 } from '@lexical/table';
 import {
   $getNearestNodeFromDOMNode,
@@ -206,11 +207,11 @@ function TableCellResizer({editor}: {editor: LexicalEditor}): JSX.Element {
             throw new Error('Expected table row');
           }
 
-          const rowCells = tableRow.getChildren();
+          const rowCells = tableRow.getChildren<TableCellNode>();
           const rowCellsSpan = rowCells.map((cell) => cell.getColSpan());
 
           const aggregatedRowSpans = rowCellsSpan.reduce(
-            (rowSpans, cellSpan) => {
+            (rowSpans: number[], cellSpan) => {
               const previousCell = rowSpans[rowSpans.length - 1] ?? 0;
               rowSpans.push(previousCell + cellSpan);
               return rowSpans;
@@ -316,14 +317,7 @@ function TableCellResizer({editor}: {editor: LexicalEditor}): JSX.Element {
 
         document.addEventListener('mouseup', mouseUpHandler(direction));
       },
-    [
-      activeCell,
-      draggingDirection,
-      resetState,
-      updateColumnWidth,
-      updateRowHeight,
-      mouseUpHandler,
-    ],
+    [activeCell, mouseUpHandler],
   );
 
   const getResizers = useCallback(() => {

diff --git a/packages/lexical-playground/src/plugins/ToolbarPlugin/index.tsx b/packages/lexical-playground/src/plugins/ToolbarPlugin/index.tsx
@@ -751,20 +751,23 @@ export default function ToolbarPlugin({
           // We split the first and last node by the selection
           // So that we don't format unselected text inside those nodes
           if ($isTextNode(node)) {
+            // Use a separate variable to ensure TS does not lose the refinement
+            let textNode = node;
             if (idx === 0 && anchor.offset !== 0) {
-              node = node.splitText(anchor.offset)[1] || node;
+              textNode = textNode.splitText(anchor.offset)[1] || textNode;
             }
             if (idx === nodes.length - 1) {
-              node = node.splitText(focus.offset)[0] || node;
+              textNode = textNode.splitText(focus.offset)[0] || textNode;
             }
 
-            if (node.__style !== '') {
-              node.setStyle('');
+            if (textNode.__style !== '') {
+              textNode.setStyle('');
             }
-            if (node.__format !== 0) {
-              node.setFormat(0);
-              $getNearestBlockElementAncestorOrThrow(node).setFormat('');
+            if (textNode.__format !== 0) {
+              textNode.setFormat(0);
+              $getNearestBlockElementAncestorOrThrow(textNode).setFormat('');
             }
+            node = textNode;
           } else if ($isHeadingNode(node) || $isQuoteNode(node)) {
             node.replace($createParagraphNode(), true);
           } else if ($isDecoratorBlockNode(node)) {

diff --git a/packages/lexical-react/src/LexicalTablePlugin.ts b/packages/lexical-react/src/LexicalTablePlugin.ts
@@ -17,6 +17,7 @@ import {useLexicalComposerContext} from '@lexical/react/LexicalComposerContext';
 import {
   $createTableCellNode,
   $createTableNodeWithDimensions,
+  $isTableCellNode,
   $isTableNode,
   applyTableHandlers,
   INSERT_TABLE_COMMAND,
@@ -176,6 +177,10 @@ export function TablePlugin({
               lastRowCell = cell;
               unmerged.push(cell);
             } else if (cell.getColSpan() > 1 || cell.getRowSpan() > 1) {
+              invariant(
+                $isTableCellNode(cell),
+                'Expected TableNode cell to be a TableCellNode',
+              );
               const newCell = $createTableCellNode(cell.__headerState);
               if (lastRowCell !== null) {
                 lastRowCell.insertAfter(newCell);