Skip to content

Commit

Permalink
Fix facebook#6409 TextNode change detection
Browse files Browse the repository at this point in the history
Add delay option to HistoryPlugin
Add test for facebook#6409
Fix lint issues
Fix $cloneWithProperties and test expectations
Targeted fix for TextNode leaf change
  • Loading branch information
etrepum committed Jul 18, 2024
1 parent 3fcf02d commit 79528d1
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 50 deletions.
6 changes: 3 additions & 3 deletions packages/lexical-clipboard/src/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ function $appendNodesToJSON(
let target = currentNode;

if (selection !== null) {
let clone = $cloneWithProperties<LexicalNode>(currentNode);
let clone = $cloneWithProperties(currentNode);
clone =
$isTextNode(clone) && selection !== null
? $sliceSelectedTextNodeContent(selection, clone)
Expand All @@ -267,11 +267,11 @@ function $appendNodesToJSON(

const serializedNode = exportNodeToJSON(target);

// TODO: TextNode calls getTextContent() (NOT node.__text) within it's exportJSON method
// TODO: TextNode calls getTextContent() (NOT node.__text) within its exportJSON method
// which uses getLatest() to get the text from the original node with the same key.
// This is a deeper issue with the word "clone" here, it's still a reference to the
// same node as far as the LexicalEditor is concerned since it shares a key.
// We need a way to create a clone of a Node in memory with it's own key, but
// We need a way to create a clone of a Node in memory with its own key, but
// until then this hack will work for the selected text extract use case.
if ($isTextNode(target)) {
const text = target.__text;
Expand Down
142 changes: 133 additions & 9 deletions packages/lexical-history/src/__tests__/unit/LexicalHistory.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import {HistoryPlugin} from '@lexical/react/LexicalHistoryPlugin';
import {RichTextPlugin} from '@lexical/react/LexicalRichTextPlugin';
import {$createQuoteNode} from '@lexical/rich-text';
import {$setBlocksType} from '@lexical/selection';
import {$restoreEditorState} from '@lexical/utils';
import {
$applyNodeReplacement,
$createNodeSelection,
$createParagraphNode,
$createRangeSelection,
Expand All @@ -26,17 +28,81 @@ import {
CAN_UNDO_COMMAND,
CLEAR_HISTORY_COMMAND,
COMMAND_PRIORITY_CRITICAL,
type KlassConstructor,
LexicalEditor,
LexicalNode,
type NodeKey,
REDO_COMMAND,
SerializedElementNode,
SerializedTextNode,
type SerializedTextNode,
type Spread,
TextNode,
UNDO_COMMAND,
} from 'lexical/src';
} from 'lexical';
import {createTestEditor, TestComposer} from 'lexical/src/__tests__/utils';
import React from 'react';
import {createRoot, Root} from 'react-dom/client';
import * as ReactTestUtils from 'shared/react-test-utils';

type SerializedCustomTextNode = Spread<
{type: ReturnType<typeof CustomTextNode.getType>; classes: string[]},
SerializedTextNode
>;

class CustomTextNode extends TextNode {
['constructor']!: KlassConstructor<typeof CustomTextNode>;

__classes: Set<string>;
constructor(text: string, classes: Iterable<string>, key?: NodeKey) {
super(text, key);
this.__classes = new Set(classes);
}
static getType(): 'custom-text' {
return 'custom-text';
}
static clone(node: CustomTextNode): CustomTextNode {
return new CustomTextNode(node.__text, node.__classes, node.__key);
}
addClass(className: string): this {
const self = this.getWritable();
self.__classes.add(className);
return self;
}
removeClass(className: string): this {
const self = this.getWritable();
self.__classes.delete(className);
return self;
}
setClasses(classes: Iterable<string>): this {
const self = this.getWritable();
self.__classes = new Set(classes);
return self;
}
getClasses(): ReadonlySet<string> {
return this.getLatest().__classes;
}
static importJSON({text, classes}: SerializedCustomTextNode): CustomTextNode {
return $createCustomTextNode(text, classes);
}
exportJSON(): SerializedCustomTextNode {
return {
...super.exportJSON(),
classes: Array.from(this.getClasses()),
type: this.constructor.getType(),
};
}
}
function $createCustomTextNode(
text: string,
classes: string[] = [],
): CustomTextNode {
return $applyNodeReplacement(new CustomTextNode(text, classes));
}
function $isCustomTextNode(
node: LexicalNode | null | undefined,
): node is CustomTextNode {
return node instanceof CustomTextNode;
}

describe('LexicalHistory tests', () => {
let container: HTMLDivElement | null = null;
let reactRoot: Root;
Expand All @@ -59,13 +125,12 @@ describe('LexicalHistory tests', () => {
// Shared instance across tests
let editor: LexicalEditor;

function TestPlugin() {
// Plugin used just to get our hands on the Editor object
[editor] = useLexicalComposerContext();
return null;
}
function Test(): JSX.Element {
function TestPlugin() {
// Plugin used just to get our hands on the Editor object
[editor] = useLexicalComposerContext();
return null;
}

return (
<TestComposer>
<RichTextPlugin
Expand Down Expand Up @@ -313,6 +378,65 @@ describe('LexicalHistory tests', () => {
await editor_.dispatchCommand(UNDO_COMMAND, undefined);
expect($isNodeSelection(editor_.getEditorState()._selection)).toBe(true);
});

test('Changes to TextNode leaf are detected properly #6409', async () => {
editor = createTestEditor({
nodes: [CustomTextNode],
});
const sharedHistory = createEmptyHistoryState();
registerHistory(editor, sharedHistory, 0);
editor.update(
() => {
$getRoot()
.clear()
.append(
$createParagraphNode().append(
$createCustomTextNode('Initial text'),
),
);
},
{discrete: true},
);
expect(sharedHistory.undoStack).toHaveLength(0);

editor.update(
() => {
// Mark dirty with no changes
for (const node of $getRoot().getAllTextNodes()) {
node.getWritable();
}
// Restore the editor state and ensure the history did not change
$restoreEditorState(editor, editor.getEditorState());
},
{discrete: true},
);
expect(sharedHistory.undoStack).toHaveLength(0);
editor.update(
() => {
// Mark dirty with text change
for (const node of $getRoot().getAllTextNodes()) {
if ($isCustomTextNode(node)) {
node.setTextContent(node.getTextContent() + '!');
}
}
},
{discrete: true},
);
expect(sharedHistory.undoStack).toHaveLength(1);

editor.update(
() => {
// Mark dirty with only a change to the class
for (const node of $getRoot().getAllTextNodes()) {
if ($isCustomTextNode(node)) {
node.addClass('updated');
}
}
},
{discrete: true},
);
expect(sharedHistory.undoStack).toHaveLength(2);
});
});

const $createParagraphNodeWithText = (text: string) => {
Expand Down
35 changes: 18 additions & 17 deletions packages/lexical-history/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,25 +193,26 @@ function isTextNodeUnchanged(

const prevSelection = prevEditorState._selection;
const nextSelection = nextEditorState._selection;
let isDeletingLine = false;

if ($isRangeSelection(prevSelection) && $isRangeSelection(nextSelection)) {
isDeletingLine =
prevSelection.anchor.type === 'element' &&
prevSelection.focus.type === 'element' &&
nextSelection.anchor.type === 'text' &&
nextSelection.focus.type === 'text';
}
const isDeletingLine =
$isRangeSelection(prevSelection) &&
$isRangeSelection(nextSelection) &&
prevSelection.anchor.type === 'element' &&
prevSelection.focus.type === 'element' &&
nextSelection.anchor.type === 'text' &&
nextSelection.focus.type === 'text';

if (!isDeletingLine && $isTextNode(prevNode) && $isTextNode(nextNode)) {
if (
!isDeletingLine &&
$isTextNode(prevNode) &&
$isTextNode(nextNode) &&
prevNode.__parent === nextNode.__parent
) {
// This has the assumption that object key order won't change if the
// content did not change, which should normally be safe given
// the manner in which nodes and exportJSON are typically implemented.
return (
prevNode.__type === nextNode.__type &&
prevNode.__text === nextNode.__text &&
prevNode.__mode === nextNode.__mode &&
prevNode.__detail === nextNode.__detail &&
prevNode.__style === nextNode.__style &&
prevNode.__format === nextNode.__format &&
prevNode.__parent === nextNode.__parent
JSON.stringify(prevEditorState.read(() => prevNode.exportJSON())) ===
JSON.stringify(nextEditorState.read(() => nextNode.exportJSON()))
);
}
return false;
Expand Down
2 changes: 1 addition & 1 deletion packages/lexical-html/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function $appendNodesToHTML(
let target = currentNode;

if (selection !== null) {
let clone = $cloneWithProperties<LexicalNode>(currentNode);
let clone = $cloneWithProperties(currentNode);
clone =
$isTextNode(clone) && selection !== null
? $sliceSelectedTextNodeContent(selection, clone)
Expand Down
3 changes: 0 additions & 3 deletions packages/lexical-react/src/LexicalContentEditable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {forwardRef, Ref, useLayoutEffect, useState} from 'react';
import {ContentEditableElement} from './shared/LexicalContentEditableElement';
import {useCanShowPlaceholder} from './shared/useCanShowPlaceholder';

/* eslint-disable @typescript-eslint/ban-types */
export type Props = Omit<ElementProps, 'editor'> & {
editor__DEPRECATED?: LexicalEditor;
} & (
Expand All @@ -31,8 +30,6 @@ export type Props = Omit<ElementProps, 'editor'> & {
}
);

/* eslint-enable @typescript-eslint/ban-types */

export const ContentEditable = forwardRef(ContentEditableImpl);

function ContentEditableImpl(
Expand Down
4 changes: 3 additions & 1 deletion packages/lexical-react/src/LexicalHistoryPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ export {createEmptyHistoryState} from '@lexical/history';
export type {HistoryState};

export function HistoryPlugin({
delay,
externalHistoryState,
}: {
delay?: number;
externalHistoryState?: HistoryState;
}): null {
const [editor] = useLexicalComposerContext();

useHistory(editor, externalHistoryState);
useHistory(editor, externalHistoryState, delay);

return null;
}
26 changes: 11 additions & 15 deletions packages/lexical-selection/src/lexical-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,37 +36,35 @@ import {
function $updateElementNodeProperties<T extends ElementNode>(
target: T,
source: ElementNode,
): T {
): void {
target.__first = source.__first;
target.__last = source.__last;
target.__size = source.__size;
target.__format = source.__format;
target.__indent = source.__indent;
target.__dir = source.__dir;
return target;
}

function $updateTextNodeProperties<T extends TextNode>(
target: T,
source: TextNode,
): T {
): void {
target.__format = source.__format;
target.__style = source.__style;
target.__mode = source.__mode;
target.__detail = source.__detail;
return target;
}

function $updateParagraphNodeProperties<T extends ParagraphNode>(
target: T,
source: ParagraphNode,
): T {
): void {
target.__textFormat = source.__textFormat;
return target;
}

/**
* Returns a copy of a node, but generates a new key for the copy.
* Returns a clone of a node with the same key and parent/next/prev pointers and other
* properties that are not set by the KlassConstructor.clone (format, style, etc.).
* @param node - The node to be cloned.
* @returns The clone of the node.
*/
Expand All @@ -79,16 +77,14 @@ export function $cloneWithProperties<T extends LexicalNode>(node: T): T {
clone.__prev = node.__prev;

if ($isElementNode(node) && $isElementNode(clone)) {
return $updateElementNodeProperties(clone, node);
}

if ($isTextNode(node) && $isTextNode(clone)) {
return $updateTextNodeProperties(clone, node);
$updateElementNodeProperties(clone, node);
if ($isParagraphNode(node) && $isParagraphNode(clone)) {
$updateParagraphNodeProperties(clone, node);
}
} else if ($isTextNode(node) && $isTextNode(clone)) {
$updateTextNodeProperties(clone, node);
}

if ($isParagraphNode(node) && $isParagraphNode(clone)) {
return $updateParagraphNodeProperties(clone, node);
}
return clone;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/lexical-website/src/components/Gallery/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ export function useFilteredExamples(examples: Array<Example>) {
searchName,
tags,
}),
[examples, searchName],
[examples, searchName, tags],
);
}

0 comments on commit 79528d1

Please sign in to comment.