Skip to content

Commit

Permalink
fix: invalidate node maps when nodes change in-between paints #5694. (#…
Browse files Browse the repository at this point in the history
…5746)

* fix: invalidate node maps when nodes change in-between paints #5694.

* Add patch changeset

* Catch additional invalide reference on Android
  • Loading branch information
DustinMackintosh authored Oct 24, 2024
1 parent 0e1e4b4 commit e97a9f8
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 37 deletions.
5 changes: 5 additions & 0 deletions .changeset/bright-fishes-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'slate-react': patch
---

Invalidate node maps when nodes change until next react paint
83 changes: 48 additions & 35 deletions packages/slate-react/src/components/editable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
} from '../utils/environment'
import Hotkeys from '../utils/hotkeys'
import {
IS_NODE_MAP_DIRTY,
EDITOR_TO_ELEMENT,
EDITOR_TO_FORCE_RENDER,
EDITOR_TO_PENDING_INSERTION_MARKS,
Expand Down Expand Up @@ -209,6 +210,11 @@ export const Editable = forwardRef(
const onDOMSelectionChange = useMemo(
() =>
throttle(() => {
if (IS_NODE_MAP_DIRTY.get(editor)) {
onDOMSelectionChange()
return
}

const el = ReactEditor.toDOMNode(editor, editor)
const root = el.getRootNode()

Expand Down Expand Up @@ -573,55 +579,62 @@ export const Editable = forwardRef(
native = false
}

// Chrome also has issues correctly editing the end of anchor elements: https://bugs.chromium.org/p/chromium/issues/detail?id=1259100
// Therefore we don't allow native events to insert text at the end of anchor nodes.
const { anchor } = selection
// If the NODE_MAP is dirty, we can't trust the selection anchor (eg ReactEditor.toDOMPoint)
if (!IS_NODE_MAP_DIRTY.get(editor)) {
// Chrome also has issues correctly editing the end of anchor elements: https://bugs.chromium.org/p/chromium/issues/detail?id=1259100
// Therefore we don't allow native events to insert text at the end of anchor nodes.
const { anchor } = selection

const [node, offset] = ReactEditor.toDOMPoint(editor, anchor)
const anchorNode = node.parentElement?.closest('a')
const [node, offset] = ReactEditor.toDOMPoint(editor, anchor)
const anchorNode = node.parentElement?.closest('a')

const window = ReactEditor.getWindow(editor)

if (
native &&
anchorNode &&
ReactEditor.hasDOMNode(editor, anchorNode)
) {
// Find the last text node inside the anchor.
const lastText = window?.document
.createTreeWalker(anchorNode, NodeFilter.SHOW_TEXT)
.lastChild() as DOMText | null
const window = ReactEditor.getWindow(editor)

if (
lastText === node &&
lastText.textContent?.length === offset
native &&
anchorNode &&
ReactEditor.hasDOMNode(editor, anchorNode)
) {
native = false
// Find the last text node inside the anchor.
const lastText = window?.document
.createTreeWalker(anchorNode, NodeFilter.SHOW_TEXT)
.lastChild() as DOMText | null

if (
lastText === node &&
lastText.textContent?.length === offset
) {
native = false
}
}
}

// Chrome has issues with the presence of tab characters inside elements with whiteSpace = 'pre'
// causing abnormal insert behavior: https://bugs.chromium.org/p/chromium/issues/detail?id=1219139
if (
native &&
node.parentElement &&
window?.getComputedStyle(node.parentElement)?.whiteSpace === 'pre'
) {
const block = Editor.above(editor, {
at: anchor.path,
match: n => Element.isElement(n) && Editor.isBlock(editor, n),
})
// Chrome has issues with the presence of tab characters inside elements with whiteSpace = 'pre'
// causing abnormal insert behavior: https://bugs.chromium.org/p/chromium/issues/detail?id=1219139
if (
native &&
node.parentElement &&
window?.getComputedStyle(node.parentElement)?.whiteSpace ===
'pre'
) {
const block = Editor.above(editor, {
at: anchor.path,
match: n => Element.isElement(n) && Editor.isBlock(editor, n),
})

if (block && Node.string(block[0]).includes('\t')) {
native = false
if (block && Node.string(block[0]).includes('\t')) {
native = false
}
}
}
}

// COMPAT: For the deleting forward/backward input types we don't want
// to change the selection because it is the range that will be deleted,
// and those commands determine that for themselves.
if (!type.startsWith('delete') || type.startsWith('deleteBy')) {
// If the NODE_MAP is dirty, we can't trust the selection anchor (eg ReactEditor.toDOMPoint via ReactEditor.toSlateRange)
if (
(!type.startsWith('delete') || type.startsWith('deleteBy')) &&
!IS_NODE_MAP_DIRTY.get(editor)
) {
const [targetRange] = (event as any).getTargetRanges()

if (targetRange) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
EDITOR_TO_PLACEHOLDER_ELEMENT,
EDITOR_TO_USER_MARKS,
IS_COMPOSING,
IS_NODE_MAP_DIRTY,
} from '../../utils/weak-maps'

export type Action = { at?: Point | Range; run: () => void }
Expand Down Expand Up @@ -345,6 +346,10 @@ export function createAndroidInputManager({
flushTimeoutId = null
}

if (IS_NODE_MAP_DIRTY.get(editor)) {
return
}

const { inputType: type } = event
let targetRange: Range | null = null
const data: DataTransfer | string | undefined =
Expand Down
7 changes: 6 additions & 1 deletion packages/slate-react/src/hooks/use-children.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import {
import ElementComponent from '../components/element'
import TextComponent from '../components/text'
import { ReactEditor } from '../plugin/react-editor'
import { NODE_TO_INDEX, NODE_TO_PARENT } from '../utils/weak-maps'
import {
IS_NODE_MAP_DIRTY,
NODE_TO_INDEX,
NODE_TO_PARENT,
} from '../utils/weak-maps'
import { useDecorate } from './use-decorate'
import { SelectedContext } from './use-selected'
import { useSlateStatic } from './use-slate-static'
Expand All @@ -36,6 +40,7 @@ const useChildren = (props: {
} = props
const decorate = useDecorate()
const editor = useSlateStatic()
IS_NODE_MAP_DIRTY.set(editor as ReactEditor, false)
const path = ReactEditor.findPath(editor, node)
const children = []
const isLeafBlock =
Expand Down
11 changes: 11 additions & 0 deletions packages/slate-react/src/plugin/with-react.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { Key } from '../utils/key'
import { findCurrentLineRange } from '../utils/lines'
import {
IS_NODE_MAP_DIRTY,
EDITOR_TO_KEY_TO_ELEMENT,
EDITOR_TO_ON_CHANGE,
EDITOR_TO_PENDING_ACTION,
Expand Down Expand Up @@ -206,6 +207,16 @@ export const withReact = <T extends BaseEditor>(

apply(op)

switch (op.type) {
case 'insert_node':
case 'remove_node':
case 'merge_node':
case 'move_node':
case 'split_node': {
IS_NODE_MAP_DIRTY.set(e, true)
}
}

for (const [path, key] of matches) {
const [node] = Editor.node(e, path)
NODE_TO_KEY.set(node, key)
Expand Down
2 changes: 1 addition & 1 deletion packages/slate-react/src/utils/weak-maps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Key } from './key'
* Two weak maps that allow us rebuild a path given a node. They are populated
* at render time such that after a render occurs we can always backtrack.
*/

export const IS_NODE_MAP_DIRTY: WeakMap<Editor, boolean> = new WeakMap()
export const NODE_TO_INDEX: WeakMap<Node, number> = new WeakMap()
export const NODE_TO_PARENT: WeakMap<Node, Ancestor> = new WeakMap()

Expand Down

0 comments on commit e97a9f8

Please sign in to comment.