Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lexical] Feature: registerMutationListener should initialize its existing nodes #6357

Merged
merged 16 commits into from
Jul 14, 2024
Merged

[lexical] Feature: registerMutationListener should initialize its existing nodes #6357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a560f2f
[lexical] registerMutationListener should initialize its existing nod…
etrepum Jun 30, 2024
c73a70a
Fix missing effect in CodeActionMenuPlugin
etrepum Jul 1, 2024
7e2d68b
Merge branch 'main' into registerMutationListener-init
etrepum Jul 1, 2024
c27ba0f
memoize expensive nodeMap calculation once per readOnly EditorState w…
etrepum Jul 2, 2024
7f86a8b
Merge branch 'main' into registerMutationListener-init
etrepum Jul 2, 2024
b5d9f54
Change default skipInitialization to be true for transitional backwar…
etrepum Jul 2, 2024
1bb7e1f
Merge remote-tracking branch 'origin/main' into registerMutationListe…
etrepum Jul 2, 2024
63723fa
Merge branch 'main' into registerMutationListener-init
etrepum Jul 3, 2024
a8bfbfb
skipInitialization false for CommentPlugin
etrepum Jul 8, 2024
9fa7bb8
Merge remote-tracking branch 'origin/main' into registerMutationListe…
etrepum Jul 8, 2024
314dfb9
Merge branch 'main' into registerMutationListener-init
etrepum Jul 12, 2024
9b19859
Do not skipInitialization for new TableHoverActions mutation listener
etrepum Jul 14, 2024
3d932c4
Merge branch 'main' into registerMutationListener-init
etrepum Jul 14, 2024
4b720a1
Fix naming typos
etrepum Jul 14, 2024
82913db
[lexical] Feature: Implement Editor.read and EditorState.read with ed…
etrepum Jul 14, 2024
47f9438
Merge branch 'main' into registerMutationListener-init
etrepum Jul 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions packages/lexical-code/src/CodeHighlighter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,18 +818,22 @@ export function registerCodeHighlighting(
}

return mergeRegister(
editor.registerMutationListener(CodeNode, (mutations) => {
editor.update(() => {
for (const [key, type] of mutations) {
if (type !== 'destroyed') {
const node = $getNodeByKey(key);
if (node !== null) {
updateCodeGutter(node as CodeNode, editor);
editor.registerMutationListener(
CodeNode,
(mutations) => {
editor.update(() => {
for (const [key, type] of mutations) {
if (type !== 'destroyed') {
const node = $getNodeByKey(key);
if (node !== null) {
updateCodeGutter(node as CodeNode, editor);
}
}
}
}
});
}),
});
},
{skipInitialization: false},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeHighlighter did not previously handle initialization correctly

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

),
editor.registerNodeTransform(CodeNode, (node) =>
codeNodeTransform(node, editor, tokenizer as Tokenizer),
),
Expand Down
46 changes: 26 additions & 20 deletions packages/lexical-playground/src/plugins/CodeActionMenuPlugin/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,26 +109,32 @@ function CodeActionMenuContainer({
};
}, [shouldListenMouseMove, debouncedOnMouseMove]);

editor.registerMutationListener(CodeNode, (mutations) => {
editor.getEditorState().read(() => {
for (const [key, type] of mutations) {
switch (type) {
case 'created':
codeSetRef.current.add(key);
setShouldListenMouseMove(codeSetRef.current.size > 0);
break;

case 'destroyed':
codeSetRef.current.delete(key);
setShouldListenMouseMove(codeSetRef.current.size > 0);
break;

default:
break;
}
}
});
});
useEffect(() => {
return editor.registerMutationListener(
CodeNode,
(mutations) => {
editor.getEditorState().read(() => {
for (const [key, type] of mutations) {
switch (type) {
case 'created':
codeSetRef.current.add(key);
break;

case 'destroyed':
codeSetRef.current.delete(key);
break;

default:
break;
}
}
});
setShouldListenMouseMove(codeSetRef.current.size > 0);
},
{skipInitialization: false},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeActionMenuPlugin did not previously handle initialization correctly

);
}, [editor]);

const normalizedLang = normalizeCodeLang(lang);
const codeFriendlyName = getLanguageFriendlyName(lang);

Expand Down
66 changes: 35 additions & 31 deletions packages/lexical-playground/src/plugins/CommentPlugin/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -840,43 +840,47 @@ export default function CommentPlugin({
});
},
),
editor.registerMutationListener(MarkNode, (mutations) => {
editor.getEditorState().read(() => {
for (const [key, mutation] of mutations) {
const node: null | MarkNode = $getNodeByKey(key);
let ids: NodeKey[] = [];

if (mutation === 'destroyed') {
ids = markNodeKeysToIDs.get(key) || [];
} else if ($isMarkNode(node)) {
ids = node.getIDs();
}

for (let i = 0; i < ids.length; i++) {
const id = ids[i];
let markNodeKeys = markNodeMap.get(id);
markNodeKeysToIDs.set(key, ids);
editor.registerMutationListener(
MarkNode,
(mutations) => {
editor.getEditorState().read(() => {
for (const [key, mutation] of mutations) {
const node: null | MarkNode = $getNodeByKey(key);
let ids: NodeKey[] = [];

if (mutation === 'destroyed') {
if (markNodeKeys !== undefined) {
markNodeKeys.delete(key);
if (markNodeKeys.size === 0) {
markNodeMap.delete(id);
ids = markNodeKeysToIDs.get(key) || [];
} else if ($isMarkNode(node)) {
ids = node.getIDs();
}

for (let i = 0; i < ids.length; i++) {
const id = ids[i];
let markNodeKeys = markNodeMap.get(id);
markNodeKeysToIDs.set(key, ids);

if (mutation === 'destroyed') {
if (markNodeKeys !== undefined) {
markNodeKeys.delete(key);
if (markNodeKeys.size === 0) {
markNodeMap.delete(id);
}
}
} else {
if (markNodeKeys === undefined) {
markNodeKeys = new Set();
markNodeMap.set(id, markNodeKeys);
}
if (!markNodeKeys.has(key)) {
markNodeKeys.add(key);
}
}
} else {
if (markNodeKeys === undefined) {
markNodeKeys = new Set();
markNodeMap.set(id, markNodeKeys);
}
if (!markNodeKeys.has(key)) {
markNodeKeys.add(key);
}
}
}
}
});
}),
});
},
{skipInitialization: false},
),
editor.registerUpdateListener(({editorState, tags}) => {
editorState.read(() => {
const selection = $getSelection();
Expand Down
26 changes: 15 additions & 11 deletions packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,21 @@ function TableActionMenu({
);

useEffect(() => {
return editor.registerMutationListener(TableCellNode, (nodeMutations) => {
const nodeUpdated =
nodeMutations.get(tableCellNode.getKey()) === 'updated';

if (nodeUpdated) {
editor.getEditorState().read(() => {
updateTableCellNode(tableCellNode.getLatest());
});
setBackgroundColor(currentCellBackgroundColor(editor) || '');
}
});
return editor.registerMutationListener(
TableCellNode,
(nodeMutations) => {
const nodeUpdated =
nodeMutations.get(tableCellNode.getKey()) === 'updated';

if (nodeUpdated) {
editor.getEditorState().read(() => {
updateTableCellNode(tableCellNode.getLatest());
});
setBackgroundColor(currentCellBackgroundColor(editor) || '');
}
},
{skipInitialization: true},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TableActionMenuPlugin did handle initialization correctly (it only listens for updates)

);
}, [editor, tableCellNode]);

useEffect(() => {
Expand Down
46 changes: 25 additions & 21 deletions packages/lexical-playground/src/plugins/TableHoverActionsPlugin/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
TableRowNode,
} from '@lexical/table';
import {$findMatchingParent, mergeRegister} from '@lexical/utils';
import {$getNearestNodeFromDOMNode} from 'lexical';
import {$getNearestNodeFromDOMNode, NodeKey} from 'lexical';
import {useEffect, useRef, useState} from 'react';
import * as React from 'react';
import {createPortal} from 'react-dom';
Expand All @@ -39,7 +39,7 @@ function TableHoverActionsContainer({
const [shouldListenMouseMove, setShouldListenMouseMove] =
useState<boolean>(false);
const [position, setPosition] = useState({});
const codeSetRef = useRef<Set<string>>(new Set());
const codeSetRef = useRef<Set<NodeKey>>(new Set());
const tableDOMNodeRef = useRef<HTMLElement | null>(null);

const debouncedOnMouseMove = useDebounce(
Expand Down Expand Up @@ -148,26 +148,30 @@ function TableHoverActionsContainer({

useEffect(() => {
return mergeRegister(
editor.registerMutationListener(TableNode, (mutations) => {
editor.getEditorState().read(() => {
for (const [key, type] of mutations) {
switch (type) {
case 'created':
codeSetRef.current.add(key);
setShouldListenMouseMove(codeSetRef.current.size > 0);
break;

case 'destroyed':
codeSetRef.current.delete(key);
setShouldListenMouseMove(codeSetRef.current.size > 0);
break;

default:
break;
editor.registerMutationListener(
TableNode,
(mutations) => {
editor.getEditorState().read(() => {
for (const [key, type] of mutations) {
switch (type) {
case 'created':
codeSetRef.current.add(key);
setShouldListenMouseMove(codeSetRef.current.size > 0);
break;

case 'destroyed':
codeSetRef.current.delete(key);
setShouldListenMouseMove(codeSetRef.current.size > 0);
break;

default:
break;
}
}
}
});
}),
});
},
{skipInitialization: false},
),
);
}, [editor]);

Expand Down
31 changes: 16 additions & 15 deletions packages/lexical-react/src/LexicalAutoEmbedPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,23 @@ export function LexicalAutoEmbedPlugin<TEmbedConfig extends EmbedConfig>({
}, []);

const checkIfLinkNodeIsEmbeddable = useCallback(
(key: NodeKey) => {
editor.getEditorState().read(async function () {
async (key: NodeKey) => {
const url = editor.getEditorState().read(function () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was very sketchy previously, and only worked at all because it used linkNode.__url. We should probably have a lint that tells you not to use async for read or update

const linkNode = $getNodeByKey(key);
if ($isLinkNode(linkNode)) {
for (let i = 0; i < embedConfigs.length; i++) {
const embedConfig = embedConfigs[i];

const urlMatch = await Promise.resolve(
embedConfig.parseUrl(linkNode.__url),
);

if (urlMatch != null) {
setActiveEmbedConfig(embedConfig);
setNodeKey(linkNode.getKey());
}
}
return linkNode.getURL();
}
});
if (url === undefined) {
return;
}
for (const embedConfig of embedConfigs) {
const urlMatch = await Promise.resolve(embedConfig.parseUrl(url));
if (urlMatch != null) {
setActiveEmbedConfig(embedConfig);
setNodeKey(key);
}
}
},
[editor, embedConfigs],
);
Expand All @@ -146,7 +145,9 @@ export function LexicalAutoEmbedPlugin<TEmbedConfig extends EmbedConfig>({
};
return mergeRegister(
...[LinkNode, AutoLinkNode].map((Klass) =>
editor.registerMutationListener(Klass, (...args) => listener(...args)),
editor.registerMutationListener(Klass, (...args) => listener(...args), {
skipInitialization: true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is not compatible with skipInitialization: false because it resets if it sees key === nodeKey which would fire with 'created', could also be fixed by checking for mutation !== 'created' && key === nodeKey

}),
),
);
}, [checkIfLinkNodeIsEmbeddable, editor, embedConfigs, nodeKey, reset]);
Expand Down
4 changes: 4 additions & 0 deletions packages/lexical-react/src/LexicalTableOfContentsPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ export function TableOfContentsPlugin({children}: Props): JSX.Element {
setTableOfContents(currentTableOfContents);
});
},
// Initialization is handled separately
{skipInitialization: true},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LexicalTableOfContentsPlugin already had an efficient and correct initialization (though this wouldn't break it)

);

// Listen to text node mutation updates
Expand All @@ -258,6 +260,8 @@ export function TableOfContentsPlugin({children}: Props): JSX.Element {
}
});
},
// Initialization is handled separately
{skipInitialization: true},
);

return () => {
Expand Down
13 changes: 1 addition & 12 deletions packages/lexical-react/src/LexicalTablePlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
$createParagraphNode,
$getNodeByKey,
$isTextNode,
$nodesOfType,
COMMAND_PRIORITY_EDITOR,
} from 'lexical';
import {useEffect} from 'react';
Expand Down Expand Up @@ -129,17 +128,6 @@ export function TablePlugin({
}
};

// Plugins might be loaded _after_ initial content is set, hence existing table nodes
// won't be initialized from mutation[create] listener. Instead doing it here,
editor.getEditorState().read(() => {
const tableNodes = $nodesOfType(TableNode);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not correct because $nodesOfType can't consider node replacement without depending on the editor

for (const tableNode of tableNodes) {
if ($isTableNode(tableNode)) {
initializeTableNode(tableNode);
}
}
});

const unregisterMutationListener = editor.registerMutationListener(
TableNode,
(nodeMutations) => {
Expand All @@ -161,6 +149,7 @@ export function TablePlugin({
}
}
},
{skipInitialization: false},
);

return () => {
Expand Down
14 changes: 7 additions & 7 deletions packages/lexical-website/docs/concepts/dom-events.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,20 @@ This can be a simple, efficient way to handle some use cases, since it's not nec

## 2. Directly Attach Handlers

In some cases, it may be better to attach an event handler directly to the underlying DOM node of each specific node. With this approach, you generally don't need to filter the event target in the handler, which can make it a bit simpler. It will also guarantee that you're handler isn't running for events that you don't care about. This approach is implemented via a [Mutation Listener](https://lexical.dev/docs/concepts/listeners).
In some cases, it may be better to attach an event handler directly to the underlying DOM node of each specific node. With this approach, you generally don't need to filter the event target in the handler, which can make it a bit simpler. It will also guarantee that your handler isn't running for events that you don't care about. This approach is implemented via a [Mutation Listener](https://lexical.dev/docs/concepts/listeners).

```js
const registeredElements: WeakSet<HTMLElement> = new WeakSet();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fixing the correctness and formatting of this relevant example, the state needs to be outside of the listener implementation!

const removeMutationListener = editor.registerMutationListener(nodeType, (mutations) => {
const registeredElements: WeakSet<HTMLElement> = new WeakSet();
editor.getEditorState().read(() => {
for (const [key, mutation] of mutations) {
const element: null | HTMLElement = editor.getElementByKey(key);
if (
// Updated might be a move, so that might mean a new DOM element
// is created. In this case, we need to add and event listener too.
(mutation === 'created' || mutation === 'updated') &&
element !== null &&
!registeredElements.has(element)
// Updated might be a move, so that might mean a new DOM element
// is created. In this case, we need to add and event listener too.
(mutation === 'created' || mutation === 'updated') &&
element !== null &&
!registeredElements.has(element)
) {
registeredElements.add(element);
element.addEventListener('click', (event: Event) => {
Expand Down
Loading
Loading