Skip to content

Commit

Permalink
Improve TypeScript types by removing [k: string]: any from LexicalNode
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@etrepum
etrepum committed Nov 10, 2023
1 parent a4f064b commit 5e311b1
Show file tree
Hide file tree
Showing 38 changed files with 293 additions and 125 deletions.
7 changes: 3 additions & 4 deletions packages/lexical-clipboard/src/clipboard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
NodeSelection,
RangeSelection,
SELECTION_CHANGE_COMMAND,
SerializedElementNode,
SerializedTextNode,
} from 'lexical';
import {CAN_USE_DOM} from 'shared/canUseDOM';
Expand Down Expand Up @@ -408,7 +409,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,
Expand All @@ -417,10 +417,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,
Expand Down
26 changes: 20 additions & 6 deletions packages/lexical-code/src/__tests__/unit/LexicalCodeNode.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import {
$getNodeByKey,
$getRoot,
$getSelection,
$isElementNode,
$isLineBreakNode,
$isRangeSelection,
$isTabNode,
$isTextNode,
$setSelection,
KEY_ARROW_DOWN_COMMAND,
KEY_ARROW_UP_COMMAND,
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -482,6 +488,7 @@ describe('LexicalCodeNode tests', () => {
'caret at start of line (first line)',
() => {
const code = $getRoot().getFirstChild();
invariant($isElementNode(code));
code.selectStart();
},
() => {
Expand Down Expand Up @@ -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);
}
},
() => {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
},
() => {
Expand Down Expand Up @@ -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);
},
() => {
Expand Down Expand Up @@ -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);
},
() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/lexical-html/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ function getConversionFunction(
if (
domConversion !== null &&
(currentConversion === null ||
currentConversion.priority < domConversion.priority)
(currentConversion.priority ?? 0) < (domConversion.priority ?? 0))
) {
currentConversion = domConversion;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/lexical-link/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export class AutoLinkNode extends LinkNode {
);
if ($isElementNode(element)) {
const linkNode = $createAutoLinkNode(this.__url, {
rel: this._rel,
rel: this.__rel,
target: this.__target,
title: this.__title,
});
Expand Down
5 changes: 5 additions & 0 deletions packages/lexical-list/src/LexicalListItemNode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,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);
}
Expand Down Expand Up @@ -185,6 +186,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);
});
Expand Down
11 changes: 9 additions & 2 deletions packages/lexical-list/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
*/

import type {LexicalNode} from 'lexical';
import type {LexicalNode, Spread} from 'lexical';

import invariant from 'shared/invariant';

Expand Down Expand Up @@ -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());
}

Expand Down
4 changes: 2 additions & 2 deletions packages/lexical-markdown/src/MarkdownImport.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
14 changes: 4 additions & 10 deletions packages/lexical-playground/src/plugins/TableCellResizer/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
$isTableCellNode,
$isTableRowNode,
getCellFromTarget,
TableCellNode,
} from '@lexical/table';
import {
$getNearestNodeFromDOMNode,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(() => {
Expand Down
17 changes: 10 additions & 7 deletions packages/lexical-playground/src/plugins/ToolbarPlugin/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,20 +757,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)) {
Expand Down
5 changes: 5 additions & 0 deletions packages/lexical-react/src/LexicalTablePlugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {useLexicalComposerContext} from '@lexical/react/LexicalComposerContext';
import {
$createTableCellNode,
$createTableNodeWithDimensions,
$isTableCellNode,
$isTableNode,
applyTableHandlers,
INSERT_TABLE_COMMAND,
Expand Down Expand Up @@ -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);
Expand Down
31 changes: 16 additions & 15 deletions packages/lexical-react/src/LexicalTreeView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
LexicalNode,
NodeSelection,
RangeSelection,
TextNode,
} from 'lexical';

import {$generateHtmlFromNodes} from '@lexical/html';
Expand Down Expand Up @@ -517,30 +518,30 @@ function printNode(node: LexicalNode) {
}

const FORMAT_PREDICATES = [
(node: LexicalNode | RangeSelection) => node.hasFormat('bold') && 'Bold',
(node: LexicalNode | RangeSelection) => node.hasFormat('code') && 'Code',
(node: LexicalNode | RangeSelection) => node.hasFormat('italic') && 'Italic',
(node: LexicalNode | RangeSelection) =>
(node: TextNode | RangeSelection) => node.hasFormat('bold') && 'Bold',
(node: TextNode | RangeSelection) => node.hasFormat('code') && 'Code',
(node: TextNode | RangeSelection) => node.hasFormat('italic') && 'Italic',
(node: TextNode | RangeSelection) =>
node.hasFormat('strikethrough') && 'Strikethrough',
(node: LexicalNode | RangeSelection) =>
(node: TextNode | RangeSelection) =>
node.hasFormat('subscript') && 'Subscript',
(node: LexicalNode | RangeSelection) =>
(node: TextNode | RangeSelection) =>
node.hasFormat('superscript') && 'Superscript',
(node: LexicalNode | RangeSelection) =>
(node: TextNode | RangeSelection) =>
node.hasFormat('underline') && 'Underline',
];

const DETAIL_PREDICATES = [
(node: LexicalNode) => node.isDirectionless() && 'Directionless',
(node: LexicalNode) => node.isUnmergeable() && 'Unmergeable',
(node: TextNode) => node.isDirectionless() && 'Directionless',
(node: TextNode) => node.isUnmergeable() && 'Unmergeable',
];

const MODE_PREDICATES = [
(node: LexicalNode) => node.isToken() && 'Token',
(node: LexicalNode) => node.isSegmented() && 'Segmented',
(node: TextNode) => node.isToken() && 'Token',
(node: TextNode) => node.isSegmented() && 'Segmented',
];

function printAllTextNodeProperties(node: LexicalNode) {
function printAllTextNodeProperties(node: TextNode) {
return [
printFormatProperties(node),
printDetailProperties(node),
Expand All @@ -560,7 +561,7 @@ function printAllLinkNodeProperties(node: LinkNode) {
.join(', ');
}

function printDetailProperties(nodeOrSelection: LexicalNode) {
function printDetailProperties(nodeOrSelection: TextNode) {
let str = DETAIL_PREDICATES.map((predicate) => predicate(nodeOrSelection))
.filter(Boolean)
.join(', ')
Expand All @@ -573,7 +574,7 @@ function printDetailProperties(nodeOrSelection: LexicalNode) {
return str;
}

function printModeProperties(nodeOrSelection: LexicalNode) {
function printModeProperties(nodeOrSelection: TextNode) {
let str = MODE_PREDICATES.map((predicate) => predicate(nodeOrSelection))
.filter(Boolean)
.join(', ')
Expand All @@ -586,7 +587,7 @@ function printModeProperties(nodeOrSelection: LexicalNode) {
return str;
}

function printFormatProperties(nodeOrSelection: LexicalNode | RangeSelection) {
function printFormatProperties(nodeOrSelection: TextNode | RangeSelection) {
let str = FORMAT_PREDICATES.map((predicate) => predicate(nodeOrSelection))
.filter(Boolean)
.join(', ')
Expand Down
Loading

0 comments on commit 5e311b1

Please sign in to comment.