Avoid overly-cautious type guards in favor of more performant type discriminators #5982

nabbydude · 2025-12-04T09:12:53Z

Description
Adds Editor.isEditorNode, Element.isElementNode, and Text.isTextNode that take a Node argument and check quickly if it's a specific node type.

Also fixes classic isEditor to actually check for all properties, actually respect deepness settings (a bug I only noticed because I was editing the file 😅), and also respect those deepness settings for its operation list.

This also removes any calls to Element.isAncestor, since it was only ever used where !Text.isTextNode would function just fine. It doesn't remove the function itself since its user accessible. Rigourously testing for any of two node types from any like this is something I cant think of a real use case for and it might be worth deprecating.

Issue
Fixes #5980
Basically checking if something is an object all the time and checking it has every single function its supposed to is a waste of performance, 99% of the time we can just quickly check if its definitely not one of the other two types and move on.

Context
This touches a lot of files, since it replaces almost every call of isEditor, isElement, and isText in the slate package.
I've removed a few redundant calls and cleaned up a few spaghetti boolean expressions.

This only changes the slate package (mostly because the packages depend on the live versions of eachother and typescript complained if I tried to call the newly added methods) the other packages can be changed once this is merged, and until then will continue to operate gracefully. Same for any user code.

Editor.isEditor previously only passed its first argument to the function it wrapped, which meant deepness options were ignored. this PR just adds that function directly to the object without wrapping it to avoid that happening again. isEditorNode is included in the same way.

I have another open PR (#5970) that changes normalizeNode, which this will conflict with. If both end up approved I have a merging commit ready.

Alternatives
I chose these names mostly to be close to the originals in alphabetical order, and to keep the isX paradigm. other options could be nodeIsEditor, isNodeEditor, Node.isEditor. I can change them if something else is preferred.

We could overwrite the old functionality and migrate the cautious type guard to verifyElement or some other name but that might break existing user code.

Future

Add docs
Use in other packages in repo
Use in examples

Checks

The new code matches the existing patterns and styles.
The tests pass with yarn test.
The linter passes with yarn lint. (Fix errors with yarn fix.)
The relevant examples still work. (Run examples with yarn start.)
You've added a changeset if changing functionality. (Add one with yarn changeset add.)

…sing them whenever possible. also switch to `node === editor` checks where it makes sense also add a comment to a confusing failure state of `Node.parent` also remove a superfluous call to `Editor.nodes` in certain codepaths of `Editor.wrapNodes`

…nstormtaylor#5307

changeset-bot · 2025-12-04T09:12:57Z

🦋 Changeset detected

Latest commit: 2f75743

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

This PR includes changesets to release 1 package

Name	Type
slate	Minor

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

nabbydude · 2025-12-04T09:21:52Z

packages/slate/src/transforms-node/merge-nodes.ts

+  return (
+    node !== editor &&
+    (Text.isTextNode(node) ||
+      Editor.isVoid(editor, node) ||
+      (node.children.length === 1 &&
+        hasSingleChildNest(editor, node.children[0])))
+  )


same functionality, rearranged for readability.
Basically its a nest if its a leaf or its only child is a leaf--and editor doesn't count.

nabbydude · 2025-12-04T09:26:54Z

packages/slate/src/transforms-node/wrap-nodes.ts


-    const roots = Array.from(
-      Editor.nodes(editor, {
-        at,
-        match: editor.isInline(element)
-          ? n => Element.isElement(n) && Editor.isBlock(editor, n)
-          : n => Editor.isEditor(n),
-        mode: 'lowest',
-        voids,
-      })
-    )
+    const roots = editor.isInline(element)
+      ? Editor.nodes(editor, {
+          at,
+          match: n => Element.isElementNode(n) && Editor.isBlock(editor, n),
+          mode: 'lowest',
+          voids,
+        })
+      : [[editor, []] as NodeEntry]



Editor.nodes with match: n => Editor.isEditor(n) only ever returns the same one node: the editor.

12joan · 2025-12-04T09:29:53Z

packages/slate/src/interfaces/node.ts

  },

  extractProps(node: Node): NodeProps {
-    if (Element.isAncestor(node)) {


Would it be worth adding an Element.isAncestorNode to make it easier to convert existing code like this?

If we did it would probably end up as just a wrapper around !Text.isTextNode (I'd assume checking string typing is faster than checking array typing although tbf I haven't checked)

I'm not personally a fan of those sorts of inverse functions-- "is not a leaf" p directly translates to "has children" in my mind. That being said it doesn't really get in my way and I don't hate it enough to boycott it if it were added.

12joan · 2025-12-04T09:37:37Z

packages/slate/src/transforms-node/unwrap-nodes.ts


      Transforms.liftNodes(editor, {
        at: range,
-        match: n => Element.isAncestor(node) && node.children.includes(n),


Same here. I find isAncestor more expressive, as it essentially means "has children".

packages/slate/src/transforms-node/wrap-nodes.ts

12joan · 2025-12-04T09:43:59Z

.changeset/rich-cheetahs-battle.md

+Add `Editor.isEditorNode`, `Element.isElementNode`, and `Text.isTextNode` as alternative type guards for when we already know the object is a node.
+Use these new functions instead of `isEditor`, `isElement`, and `isText` whenever possible, the classic functions are only necessary for typechecking an entirely unknown object.


Nice! Should we also change the docs to refer to the new type guards by default?

I definitely think so, user code should only need the old functions for serializing/deserializing etc, basically call match functions etc are better off with the new ones.

Just didn't want to cement them in too deep before getting feedback on the naming. Thankfully old code still functions, just inefficiently (equally as inefficiently as before, to be precise)

packages/slate/src/editor/is-editor.ts

12joan · 2025-12-04T10:05:52Z

packages/slate/src/editor/is-editor.ts

    return false
  }

  const isEditor =


I'm not sure whether it's worth including this many typeof value.x === 'function' checks. Each one carries a small overhead, and there's a big difference between having 16 of them versus 86 (apologies if I miscounted).

Here's a benchmark I ran to compare these numbers of typeof checks. Since the numbers don't mean much in isolation, I also included a WeakMap.prototype.set call for comparison. Setting and getting weak map values tends to be relatively slow, so anything slower than it should generally be avoided if possible.

https://jsbm.dev/xM6mIrc116gyW - Longer bars are faster.

Firefox Chrome Safari

Yeah, my intention would be that Editor.isEditor is basically never used. The only current use case I can imagine in the current codebase is maybe the sanity check that slate-react's Slate component does when its first passed an editor prop but really that should be unnecessary in a typescripted world.

As I'm typing though I do see the perspective of any user code that updates its dependency without tweaking its calls getting an unexpected performance hit. 🤔

12joan · 2025-12-04T10:12:15Z

This only changes the slate package (mostly because the packages depend on the live versions of eachother and typescript complained if I tried to call the newly added methods) the other packages can be changed once this is merged, and until then will continue to operate gracefully. Same for any user code.

The other packages don't use the live versions; I believe they use the transpiled versions of the packages found in dist. There should be a command you can run to automatically keep these dist versions up to date. yarn watch, I think.

Bear in mind that if you do update the other packages to depend on the new functions, you'll also need to bump the peer dependency versions.

12joan · 2025-12-04T10:13:07Z

We could overwrite the old functionality and migrate the cautious type guard to verifyElement or some other name but that might break existing user code.

I would be in favour of this personally, but I'd be interested to hear what others think.

nabbydude · 2025-12-04T22:02:01Z

The other packages don't use the live versions; I believe they use the transpiled versions of the packages found in dist. There should be a command you can run to automatically keep these dist versions up to date. yarn watch, I think.

Well that'll show me assuming repos other people create are as jank as my own. In that case I can add the other packages changes to this PR

nabbydude · 2025-12-05T04:33:22Z

site/examples/js/inlines.jsx

-      onMouseDown={event => {
-        event.preventDefault()
+      onPointerDown={event => event.preventDefault()}
+      onClick={() => {


these were changed by yarn tsc:examples, they already existed in the tsx files and weren't synced

nabbydude · 2025-12-05T06:29:51Z

replaced the calls in other packages as well. A few were left in intentionally:

https://github.com/nabbydude/slate/blob/2f75743c89b7561d95b1b5cd8cd5a88f7f5dc925/packages/slate-react/src/components/slate.tsx#L47-L51
This check gets called when a <Slate> component gets first mounted. I'd be in favor of this check being deleted entirely because if typescript is being respected it never ever happens, but it isn't a hot path so for now I'm just assuming it's here on purpose for baby-proofing.

https://github.com/nabbydude/slate/blob/2f75743c89b7561d95b1b5cd8cd5a88f7f5dc925/packages/slate-hyperscript/src/creators.ts#L35
This is being received as any by the surrounding function and I don't have enough familiarity with slate-hyperscript to write more sensible type guards so I'll assume it could be necessary and leave it for now.

I also didn't touch any of the tests, since rigorous checks have purpose there.

nabbydude added 5 commits December 4, 2025 01:36

fix deep checking for isEditor and check for new properties from ia…

4c2b6b7

…nstormtaylor#5307

Merge branch 'ianstormtaylor:main' into is-editor-fixes

8826be3

convert all instances of Element.isAncestor and a few I missed before

83c75ff

changeset

a8b11cc

nabbydude changed the title ~~Remove overly cautios type guards in favor of more performant type discriminators~~ Remove overly-cautious type guards in favor of more performant type discriminators Dec 4, 2025

nabbydude changed the title ~~Remove overly-cautious type guards in favor of more performant type discriminators~~ Avoid overly-cautious type guards in favor of more performant type discriminators Dec 4, 2025

nabbydude commented Dec 4, 2025

View reviewed changes

fix pretter complaint in changeset

da7cf68

12joan suggested changes Dec 4, 2025

View reviewed changes

nabbydude added 2 commits December 4, 2025 23:05

use new guards in other packages and examples

fd0ddc7

revert wrap nodes change (keeping new guards)

2f75743

nabbydude commented Dec 5, 2025

View reviewed changes

		Add `Editor.isEditorNode`, `Element.isElementNode`, and `Text.isTextNode` as alternative type guards for when we already know the object is a node.
		Use these new functions instead of `isEditor`, `isElement`, and `isText` whenever possible, the classic functions are only necessary for typechecking an entirely unknown object.

Avoid overly-cautious type guards in favor of more performant type discriminators #5982

Are you sure you want to change the base?

Avoid overly-cautious type guards in favor of more performant type discriminators #5982

Conversation

nabbydude commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

12joan Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

12joan commented Dec 4, 2025

Uh oh!

12joan commented Dec 4, 2025

Uh oh!

nabbydude commented Dec 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nabbydude commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nabbydude commented Dec 4, 2025 •

edited

Loading

changeset-bot bot commented Dec 4, 2025 •

edited

Loading

12joan Dec 4, 2025 •

edited

Loading