-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
[Breaking Change][lexical] Feature: New update tag: skip-dom-selection, $onUpdate now always called #6894
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
So what's currently happening here is:
|
Autocomplete is a necessary condition for this, so it seems that interaction between autocomplete and collab is the real problem here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is passing all tests now, just needs a re-approval. Added some commentary on the new changes.
@@ -52,7 +52,12 @@ test.describe('Autocomplete', () => { | |||
class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr" | |||
dir="ltr"> | |||
<span data-lexical-text="true">Sort by alpha</span> | |||
<span data-lexical-text="true"></span> | |||
<span |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the autocomplete node to render more than an empty span in collab, but set its display to none. This makes it easier to see what's going on in the DOM. I think ultimately there should be some other solution for autocomplete in collab that doesn't get synchronized.
@@ -960,7 +970,10 @@ export class LexicalEditor { | |||
registeredNodes.push(registeredReplaceWithNode); | |||
} | |||
|
|||
markAllNodesAsDirty(this, klass.getType()); | |||
markNodesWithTypesAsDirty( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed this internal function because now it actually does what we need it to do
@@ -607,7 +607,8 @@ export function $commitPendingUpdates( | |||
editor._editable && | |||
// domSelection will be null in headless | |||
domSelection !== null && | |||
(needsUpdate || pendingSelection === null || pendingSelection.dirty) | |||
(needsUpdate || pendingSelection === null || pendingSelection.dirty) && | |||
!tags.has('skip-dom-selection') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the logic that skips DOM selection with that tag
@@ -1005,6 +1006,7 @@ function $beginUpdate( | |||
|
|||
const shouldUpdate = | |||
editor._dirtyType !== NO_DIRTY_NODES || | |||
editor._deferred.length > 0 || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where we defer the update whenever there is an onUpdate, so you can attach the callback, do some mutations in another update issued that tick, and still get a callback
@@ -72,7 +72,6 @@ import { | |||
internalGetActiveEditorState, | |||
isCurrentlyReadOnlyMode, | |||
triggerCommandListeners, | |||
updateEditor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see a reason to use this internal function here
// Mark all existing text nodes as dirty | ||
updateEditor( | ||
editor, | ||
export function markNodesWithTypesAsDirty( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously whenever a transform was registered we marked all nodes in the editor as dirty, which was surprisingly the biggest factor in the collab+autocomplete+selection issue. Now it only marks relevant nodes as dirty, which in this case is none. We can use the same internal facility developed for registerMutationListener initialization to find the matching nodes by type in the reconciled editor state to determine which ones should be marked as dirty.
node.markDirty(); | ||
for (const nodeMap of dirtyNodeMaps) { | ||
for (const node of nodeMap.values()) { | ||
node.markDirty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to check if the nodes are attached, worst case they just end up being re-inserted in the nodeMap and get GC'd on reconciliation.
@@ -1825,17 +1833,26 @@ export function getCachedTypeToNodeMap( | |||
); | |||
let typeToNodeMap = cachedNodeMaps.get(editorState); | |||
if (!typeToNodeMap) { | |||
typeToNodeMap = new Map(); | |||
typeToNodeMap = computeTypeToNodeMap(editorState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just extracting some logic to a function, nothing was changed here, I thought I might need to do it with a live EditorState but then realized that the frozen one will work even better since all nodes in the live editorstate are either in the frozen one or are already dirty.
@@ -29,6 +29,7 @@ export type { | |||
SerializedEditor, | |||
Spread, | |||
Transform, | |||
UpdateListener, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I was going to need to use tags in the updateListener in the Autocomplete plugin and noticed this type was not exported
…ts context Changing the focus behavior was more difficult than expected. Apparently when Lexical has a Selection then it will 'steal' the focus whenever changes to its content is made. There have been made changes to Lexical just recently that would allow us to avoid this (facebook/lexical#6894) but we haven't updated to this version yet and updating this closely before the EAP seems risky. I found a workaround in one of the discussions that suggests to set the selection to `null` when making a change. In order to do this I refactored some of the function to make them more composable. Instead of calling `editor.update` directly these functions are now supposed to be called from `editor.update`. Since I've added a helper for creating a promise-version of `editor.update` anyway, I also addressed one of the issues I've encountered in the last PR, namely that `onUpdate` is not called when the editor state doesn't change. This will btw also be fixed on the Lexical side by the above mentioned PR.
…ts context (#6385) Closes srch-1483 Changing the focus behavior was more difficult than expected. Apparently when Lexical has a Selection then it will 'steal' the focus whenever changes to its content is made. There have been made changes to Lexical just recently that would allow us to avoid this (facebook/lexical#6894) but we haven't updated to this version yet and updating this closely before the EAP seems risky. I found a workaround in one of the discussions that suggests to set the selection to `null` when making a change. In order to do this I refactored some of the function to make them more composable. Instead of calling `editor.update` directly these functions are now supposed to be called from `editor.update`. Since I've added a helper for promisifying `editor.update` anyway, I also addressed one of the issues I've encountered in the last PR, namely that `onUpdate` is not called when the editor state doesn't change. To prevent accidentally calling it in a way that would not resolve the promise, the callback has to return a boolean. This will btw also be fixed on the Lexical side by the above mentioned PR. ## Test plan Manual testing https://github.com/user-attachments/assets/296cca9e-b8e3-433e-a39a-befbb8fe2017
…ts context (#6385) Closes srch-1483 Changing the focus behavior was more difficult than expected. Apparently when Lexical has a Selection then it will 'steal' the focus whenever changes to its content is made. There have been made changes to Lexical just recently that would allow us to avoid this (facebook/lexical#6894) but we haven't updated to this version yet and updating this closely before the EAP seems risky. I found a workaround in one of the discussions that suggests to set the selection to `null` when making a change. In order to do this I refactored some of the function to make them more composable. Instead of calling `editor.update` directly these functions are now supposed to be called from `editor.update`. Since I've added a helper for promisifying `editor.update` anyway, I also addressed one of the issues I've encountered in the last PR, namely that `onUpdate` is not called when the editor state doesn't change. To prevent accidentally calling it in a way that would not resolve the promise, the callback has to return a boolean. This will btw also be fixed on the Lexical side by the above mentioned PR. ## Test plan Manual testing https://github.com/user-attachments/assets/296cca9e-b8e3-433e-a39a-befbb8fe2017
commit a62a1a6 Author: James Fitzsimmons <[email protected]> Date: Thu Jan 30 19:13:35 2025 +1100 [lexical-mark] Feature: include inline decorator nodes in marks (facebook#7086) commit 881c7fe Author: Bob Ippolito <[email protected]> Date: Thu Jan 30 00:13:00 2025 -0800 [lexical-utils] Fix: Modify $reverseDfs to be a right-to-left variant of $dfs (facebook#7112) commit ce2bb45 Author: Nigel Gutzmann <[email protected]> Date: Wed Jan 29 14:41:12 2025 -0800 [lexical-utils] Feature: add reverse dfs iterator (facebook#7107) commit 3a140d2 Author: mohammed shaheer kp <[email protected]> Date: Tue Jan 28 06:19:45 2025 +0530 [lexical-playground] Bug Fix: Ensure Delete Node handles all node types (facebook#7096) Co-authored-by: shaheerkpzaigo <[email protected]> commit 8e2ede2 Author: Adam Pugh <[email protected]> Date: Mon Jan 27 18:49:38 2025 -0600 Listeners Lexical: 3 updates to spelling and grammar - Update listeners.md (facebook#7100) commit 9fcc494 Author: Adam Pugh <[email protected]> Date: Mon Jan 27 18:49:34 2025 -0600 Lexical Docs: 2 updates to spelling README.md (facebook#7102) commit 946a6df Author: Adam Pugh <[email protected]> Date: Mon Jan 27 18:49:29 2025 -0600 Selection | Lexical: 1 Spelling Update Update selection.md (facebook#7103) commit ce93ea6 Author: Adam Pugh <[email protected]> Date: Mon Jan 27 18:49:25 2025 -0600 Creating a React Plugin: 1 Grammar Update - Update create_plugin.md (facebook#7104) commit ed29d89 Author: Adam Pugh <[email protected]> Date: Mon Jan 27 18:49:21 2025 -0600 Working with DOM Events: 2 Spelling and Grammar Updates Update dom-ev… (facebook#7105) commit 212b70f Author: James Fitzsimmons <[email protected]> Date: Mon Jan 27 08:48:09 2025 +1100 [lexical-yjs] Bug Fix: handle text node being split by Yjs redo (facebook#7098) commit 6a98a47 Author: Torleif Berger <[email protected]> Date: Fri Jan 24 21:46:45 2025 +0100 [lexical-react] Bug Fix: Import `JSX` type from React to prevent "Cannot find namespace 'JSX'"-error when type-checking with React 19 (facebook#7080) commit f8e5968 Author: Tetsuya <[email protected]> Date: Sat Jan 25 04:06:57 2025 +0800 [lexical] Chore: Rename variable and add comments for Safari compositing workaround (facebook#7092) commit 81c9ab6 Author: Mateo Vuković <[email protected]> Date: Fri Jan 24 18:44:05 2025 +0100 Fix: Use already defined RegisteredNodes type (facebook#7085) commit 63958a2 Author: Sherry <[email protected]> Date: Tue Jan 21 18:15:21 2025 +0800 [playground] Bug fix: prevent growing whitespaces in markdown table toggle (facebook#7041) Co-authored-by: Bob Ippolito <[email protected]> commit d9f9924 Author: Sherry <[email protected]> Date: Tue Jan 21 14:58:08 2025 +0800 Unrevert [Breaking Change][lexical] Bug Fix: Commit updates on editor.setRootElement(null) facebook#7023 (facebook#7068) commit 92fa0a3 Author: mohammed shaheer kp <[email protected]> Date: Tue Jan 21 06:23:24 2025 +0530 [lexical-playground] plugins TableOfContent Scroll smooth behaviour A… (facebook#7069) commit 2e4a63e Author: Ivaylo Pavlov <[email protected]> Date: Mon Jan 20 02:37:34 2025 +0000 [lexical-playground] Fix Columns Layout Item Overflow (facebook#7066) commit d319b07 Author: Bob Ippolito <[email protected]> Date: Sun Jan 19 14:45:41 2025 -0800 Change fork modules to use production only when NODE_ENV explicitly set to production (facebook#7065) commit 46c9c2f Author: CityHunter <[email protected]> Date: Sat Jan 18 13:00:38 2025 +0800 [lexical] Bug Fix: In the Safari browser, during the compositing event process, the delete key exhibits unexpected behavior. (facebook#7061) Co-authored-by: 涂博闻 <[email protected]> commit 92a1cd7 Author: Violet Rosenzweig <[email protected]> Date: Thu Jan 16 18:44:11 2025 -0500 docs: Change "here" link to more descriptive text (facebook#7058) commit f6377a3 Author: Aman Harwara <[email protected]> Date: Fri Jan 17 02:08:17 2025 +0530 [lexical-table] Bug Fix: Prevent error if pasted table has empty row (facebook#7057) commit 0835029 Author: Aman Harwara <[email protected]> Date: Fri Jan 17 00:18:08 2025 +0530 [lexical-list] Bug Fix: Prevent error when calling formatList when selection is at root (facebook#6994) commit 940435d Author: Brayden <[email protected]> Date: Wed Jan 15 16:10:01 2025 -0800 fix: iOS Autocorrect strips formatting by reporting wrong dataType (facebook#5789) Co-authored-by: Bob Ippolito <[email protected]> commit 136a565 Author: Aman Harwara <[email protected]> Date: Thu Jan 16 04:48:32 2025 +0530 [lexical-yjs] Feature: Allow passing in custom `syncCursorPositions` function to collab hook (facebook#7053) commit 415c576 Author: Maksim Horbachevsky <[email protected]> Date: Wed Jan 15 18:18:03 2025 -0500 fix: triple click around inline elements (links) (facebook#7055) commit a3ef4f3 Author: Ivaylo Pavlov <[email protected]> Date: Wed Jan 15 23:15:39 2025 +0000 [lexical-table] Support table alignment (facebook#7044) commit 29d733c Author: Sherry <[email protected]> Date: Wed Jan 15 21:50:07 2025 +0800 Revert [Breaking Change][lexical] Bug Fix: Commit updates on editorSetRootElement(null) (facebook#7023) (facebook#7052) commit 65ce66a Author: Bob Ippolito <[email protected]> Date: Tue Jan 14 14:57:54 2025 -0800 [lexical] Bug Fix: Normalize selection after applyDOMRange to account for Firefox differences (facebook#7050) commit bbc07af Author: Bob Ippolito <[email protected]> Date: Tue Jan 14 08:55:46 2025 -0800 [*] Bug Fix: Use GITHUB_OUTPUT instead of GITHUB_ENV for size-limit action (facebook#7051) commit c8f27ed Author: Bob Ippolito <[email protected]> Date: Tue Jan 14 06:36:13 2025 -0800 [Breaking Change][*] Chore: Use terser for optimizing cjs prod build (facebook#7047) commit 8bd22d5 Author: Bob Ippolito <[email protected]> Date: Mon Jan 13 07:09:31 2025 -0800 [lexical] Bug Fix: Handle MutationObserver/input event re-ordering when using contentEditable inside of an iframe (facebook#7045) commit 930629c Author: Ivaylo Pavlov <[email protected]> Date: Sat Jan 11 06:03:30 2025 +0000 Clean up nested editor update (facebook#7039) commit bd874a3 Author: Bob Ippolito <[email protected]> Date: Fri Jan 10 15:23:54 2025 -0800 [Breaking Change][lexical][lexical-selection][lexical-list] Bug Fix: Fix infinite loop when splitting invalid ListItemNode (facebook#7037) commit 541fa43 Author: Bob Ippolito <[email protected]> Date: Thu Jan 9 12:42:23 2025 -0800 v0.23.1 (facebook#7035) Co-authored-by: Lexical GitHub Actions Bot <> commit d7abafd Author: Bob Ippolito <[email protected]> Date: Thu Jan 9 08:33:12 2025 -0800 [Breaking Change][lexical] Bug Fix: Commit updates on editor.setRootElement(null) (facebook#7023) commit 6add515 Author: Bob Ippolito <[email protected]> Date: Wed Jan 8 17:27:15 2025 -0800 [lexical] Fix TabNode deserialization regression (facebook#7031) commit 33e3677 Author: Maksim Horbachevsky <[email protected]> Date: Wed Jan 8 14:59:03 2025 -0500 [lexical-react] Feature: Merge TabIndentionPlugin and ListMaxIndentLevelPlugin plugins (facebook#7018) commit 7de86e4 Author: James Fitzsimmons <[email protected]> Date: Wed Jan 8 09:45:25 2025 +1100 [lexical-mark] Bug Fix: reverse ternary in MarkNode.addID (facebook#7020) commit 7961130 Author: Bob Ippolito <[email protected]> Date: Sun Jan 5 13:55:25 2025 -0800 v0.23.0 (facebook#7017) Co-authored-by: Lexical GitHub Actions Bot <> commit 2b4252d Author: Aman Harwara <[email protected]> Date: Sat Jan 4 11:31:19 2025 +0530 [lexical-yjs] Feature: Expose function to get anchor and focus nodes for given user awareness state (facebook#6942) commit 8100d6d Author: Ivaylo Pavlov <[email protected]> Date: Sat Jan 4 01:12:04 2025 +0000 [lexical-playground] Fix table hover actions button position (facebook#7011) commit bd1ef2a Author: Bob Ippolito <[email protected]> Date: Fri Jan 3 14:25:31 2025 -0800 [lexical] Bug Fix: Fix registerNodeTransform regression introduced in facebook#6894 (facebook#7016) commit 85c08b6 Author: Christian Grøngaard <[email protected]> Date: Thu Jan 2 00:20:20 2025 +0100 [lexical-playground] Refactor: switch headings test file names (facebook#7008) commit 7c21d4f Author: Bob Ippolito <[email protected]> Date: Wed Jan 1 12:48:12 2025 -0800 [Breaking Change][lexical] Feature: Add updateFromJSON and move more textFormat/textStyle to ElementNode (facebook#6970) commit aaa9009 Author: Bob Ippolito <[email protected]> Date: Wed Jan 1 07:50:39 2025 -0800 [lexical] Bug Fix: Fix getNodes over-selection (facebook#7006) commit 803391d Author: Sherry <[email protected]> Date: Tue Dec 31 11:26:17 2024 +0800 [__test__] npm upgrade astro (facebook#7001) commit 684352b Author: Christian Grøngaard <[email protected]> Date: Mon Dec 30 05:12:45 2024 +0100 Documentation: Fix typo "nest nest"->"nest" in README.md (facebook#7000) Co-authored-by: Bob Ippolito <[email protected]> commit 27b75cc Author: Sherry <[email protected]> Date: Fri Dec 27 11:06:29 2024 +0800 [__tests__] npm upgrade next (facebook#6996) commit 05ddbcc Author: Simon <[email protected]> Date: Thu Dec 26 03:37:50 2024 +0100 [lexical] Bug Fix: Flow is missing some variables and functions (facebook#6977) commit e79c946 Author: Sherry <[email protected]> Date: Tue Dec 24 09:54:46 2024 +0800 v0.22.0 (facebook#6993) Co-authored-by: Lexical GitHub Actions Bot <> commit c415f7a Author: Sam Zhou <[email protected]> Date: Mon Dec 23 10:31:36 2024 -0800 [lexical-react] Refactor: Replace `React$MixedElement` and `React$Node` with `React.MixedElement` and `React.Node` (facebook#6984) commit c844a4d Author: Sherry <[email protected]> Date: Tue Dec 24 02:30:52 2024 +0800 [lexical] Fix flow error: change this to any (facebook#6992) commit 6190033 Author: Germán Jabloñski <[email protected]> Date: Mon Dec 23 05:19:27 2024 -0300 Refactor: exportJSON (facebook#6983) commit e0dafb8 Author: Germán Jabloñski <[email protected]> Date: Sat Dec 21 13:59:01 2024 -0300 feature: expose forEachSelectedTextNode (facebook#6981) Co-authored-by: Bob Ippolito <[email protected]> commit 23715f5 Author: Alex <[email protected]> Date: Fri Dec 20 18:23:27 2024 +0300 [lexical][lexical-table] Bug fix: TablePlugin: - check is current selection in target table node (facebook#6979) Co-authored-by: alazarev <[email protected]>
Description
'skip-dom-selection'
update tag. Both of these are relevant to a very common FAQ which is about how to avoid the editor taking over the selection with a programmatic update.updateDOM
to be more correct, theprevNode
argument should always bethis
(the same class, but a different instance). This fixes some subclassing issues.onUpdate
and$onUpdate
to always be called. Previously they would be skipped if the update containing them did not make any nodes dirty. It's often useful to use$onUpdate
to attach some logic in adispatchCommand
that does not need to update nodes or the selection but needs to do something with the reconciled DOM or editor afterwards.Closes #6514
Clsoes #4474
Upgrade Notes
If you have code that expects
onUpdate
or$onUpdate
to not be called if the update did not make any nodes or the selection dirty, then you will have to change your code accordingly.Now when registerNodeTransform is called, only relevant nodes (by type, including support for withKlass) are marked dirty. Previously, all nodes in the editor were marked as dirty when any transform was registered (except RootNode, which was a special case).
Test plan
Unit tests were added for the new behavior of
onUpdate
/$onUpdate
and the'skip-dom-selection'
tag.