Skip to content

Conversation

delijah
Copy link
Collaborator

@delijah delijah commented Aug 6, 2025

Checklist

  • yarn typecheck
  • yarn lint:fix
  • yarn test
  • yarn brl
  • yarn changeset
  • ui changelog

This implements a useNodePath hook, which will trigger a re-render, whenever a path of a node has been changed.

Copy link

codesandbox bot commented Aug 6, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

changeset-bot bot commented Aug 6, 2025

🦋 Changeset detected

Latest commit: e4291ad

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 43 packages
Name Type
@platejs/core Major
platejs Major
@platejs/utils Major
@platejs/autoformat Major
@platejs/basic-nodes Major
@platejs/basic-styles Major
@platejs/callout Major
@platejs/caption Major
@platejs/code-block Major
@platejs/combobox Major
@platejs/comment Major
@platejs/csv Major
@platejs/cursor Major
@platejs/date Major
@platejs/diff Major
@platejs/dnd Major
@platejs/docx Major
@platejs/emoji Major
@platejs/excalidraw Major
@platejs/find-replace Major
@platejs/floating Major
@platejs/indent Major
@platejs/juice Major
@platejs/layout Major
@platejs/link Major
@platejs/list-classic Major
@platejs/list Major
@platejs/markdown Major
@platejs/math Major
@platejs/media Major
@platejs/mention Major
@platejs/playwright Major
@platejs/resizable Major
@platejs/selection Major
@platejs/slash-command Major
@platejs/suggestion Major
@platejs/tabbable Major
@platejs/table Major
@platejs/tag Major
@platejs/toc Major
@platejs/toggle Major
@platejs/yjs Major
@platejs/ai Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Aug 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plate ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2025 1:52pm

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 6, 2025
@dosubot dosubot bot added the core label Aug 6, 2025
@zbeyens
Copy link
Member

zbeyens commented Aug 6, 2025

Can't we use useEditorSelector instead of overriding apply?

@delijah
Copy link
Collaborator Author

delijah commented Aug 6, 2025

Can't we use useEditorSelector instead of overriding apply?

If i understand correctly, we could only select props from the editor object, and trigger re-renders, whenever any node value or the editors selection changed, when using useEditorSelector.

My version is more fine grained, it will only react to node operations, not to text operations, nor to selection operations. Also it will react faster, immediately after and operation was applied, it doesn't wait for operations to be flushed to the onChange event. It also makes use of the PathRef feature, which belongs to the slate core and should be really performant.

But after this has been said, i am not too familiar yet with the useEditorSelector capabilities yet. Maybe there would be a way of reaching similar performance? But as far as i saw, it can only react to onChange, 'onValueChange, onSelectionChangeand to changes of props of theeditor` object.

@delijah
Copy link
Collaborator Author

delijah commented Aug 7, 2025

What could be quite nice, would actually be something like that:

import { useEffect, useMemo, useState } from 'react';
import { Editor, type Node } from 'slate';
import { ReactEditor, useSlateStatic } from 'slate-react';

export const useNodePath = (node: Node) => {
    const editor = useSlateStatic();
    const [, setCacheKey] = useState(0);

    const pathRef = useMemo(() => {
        const path = ReactEditor.findPath(editor, node);

        if (path) {
            return Editor.pathRef(editor, path, {
                affinity: 'backward',
                onChange: () => {
                    setCacheKey((prev) => prev + 1);
                },
            });
        }

        return {
            current: null,
            unref: () => {},
        };
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [node]);

    useEffect(
        () => () => {
            pathRef.unref();
        },
        [pathRef],
    );

    return pathRef.current;
};

Of course the change handler on pathRefs would have to be implemented by slate-react.

Actually the hook could go into that package then. Or we could even think about a hook called usePathRef.

What do you think about that @zbeyens ?

@delijah
Copy link
Collaborator Author

delijah commented Aug 7, 2025

Something like that: ianstormtaylor/slate#5930

@zbeyens
Copy link
Member

zbeyens commented Aug 7, 2025

Yes that's better. I'm working on the onNodeChange prop right now.

@delijah
Copy link
Collaborator Author

delijah commented Aug 7, 2025

I get the feeling, that using the following might be sufficient 😅

const key = editor.api.findKey(element);

const { path, selection } = useEditorSelector(
    (editor) => ({
        path: editor.api.findPath(element),
        selection: editor.selection,
    }),
    [key],
    {
        equalityFn: (a, b) => {
            const isPathEqual =
                a.path && b.path ? PathApi.equals(a.path, b.path) : a.path === b.path;

            const isSelectionEqual =
                a.selection && b.selection
                    ? RangeApi.equals(a.selection, b.selection)
                    : a.selection === b.selection;

            return isPathEqual && isSelectionEqual;
        },
    },
);

@zbeyens
Copy link
Member

zbeyens commented Aug 7, 2025

Perhaps we could add more parameters like operations (not operation, since onChange can be called after many operations) to useEditorSelector so this could be optimized to a subset of operations. This needs to be patched first though ianstormtaylor/slate#5863 (comment)

@delijah
Copy link
Collaborator Author

delijah commented Aug 29, 2025

Perhaps we could add more parameters like operations (not operation, since onChange can be called after many operations) to useEditorSelector so this could be optimized to a subset of operations. This needs to be patched first though ianstormtaylor/slate#5863 (comment)

Couldn't we just use editor.operations for that?

@zbeyens
Copy link
Member

zbeyens commented Aug 29, 2025

Yes if that's synced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants