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.

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

dispatch=undefined in createCan() results in dispatch attempts during can() #3025

Closed
2 tasks done
kivikakk opened this issue Jul 26, 2022 · 0 comments · Fixed by #3026
Closed
2 tasks done

dispatch=undefined in createCan() results in dispatch attempts during can() #3025

kivikakk opened this issue Jul 26, 2022 · 0 comments · Fixed by #3026
Labels
Type: Bug The issue or pullrequest is related to a bug

Comments

@kivikakk
Copy link
Contributor

What’s the bug you are facing?

I've hit a weird issue that requires a combination of three things:

  1. A custom node that nests another custom node, which in turn contains inlines.
    a. It needs to have higher priority than Paragraph.
  2. editor.can().toggleBulletList() (or editor.can().toggleOrderedList())
  3. A selection from within a paragraph after the custom node into the inline in the custom node.

Which browser was this experienced in? Are any special extensions installed?

Latest Chrome on macOS, but unlikely to be browser-specific.

How can we reproduce the bug on our side?

Here's a sandbox those shows the issue: https://codesandbox.io/s/tiptap-dispatch-repro-gc54w8?file=/src/App.js

Try making a selection like this:

image

editor?.can().toggleBulletList() will cause a RangeError, specifically from here:

if (node.type.isTextblock) {
const { defaultType } = $mappedFrom.parent.contentMatchAt($mappedFrom.index())
tr.setNodeMarkup(nodeRange.start, defaultType)
}

The tr.setNodeMarkup call throws:

RangeError
Invalid content for node type mycontainer

See relevant PM code. At the point of error, type is the NodeType for MyContainer, and node.content is a fragment containing TextNode(s).

Can you provide a CodeSandbox?

No response

What did you expect to happen?

I'm surprised that a can() call could throw an error at all, regardless of the schema. clearNodes checks for whether it should dispatch:

if (!dispatch) {
return true
}

But in this case, dispatch is () => void 0. That value comes from CommandManager.buildProps:

dispatch: shouldDispatch
? () => undefined
: undefined,

shouldDispatch is true. This is surprising to me. Here's the relevant part of CommandManager.createCan:

public createCan(startTr?: Transaction): CanCommands {
const { rawCommands, state } = this
const dispatch = undefined
const tr = startTr || state.tr
const props = this.buildProps(tr, dispatch)

dispatch is set to undefined. Even explicitly passed undefined arguments are subject to defaults (today I learned :/ apparently JavaScript can still surprise me), which means the this.buildProps(tr, dispatch) call gets shouldDispatch set to true.

A year and a half ago in ca8d1a4 the dispatch value in createCan was changed from false to undefined:

image

That led to this state of affairs, I think.

Anything to add? (optional)

The following appears to fix this issue, and all tests pass locally, so I'll open a PR:

diff --git a/packages/core/src/CommandManager.ts b/packages/core/src/CommandManager.ts
index c1c62ffcc..7e4dd561d 100644
--- a/packages/core/src/CommandManager.ts
+++ b/packages/core/src/CommandManager.ts
@@ -107,13 +107,13 @@ export class CommandManager {

   public createCan(startTr?: Transaction): CanCommands {
     const { rawCommands, state } = this
-    const dispatch = undefined
+    const dispatch = false
     const tr = startTr || state.tr
     const props = this.buildProps(tr, dispatch)
     const formattedCommands = Object.fromEntries(Object
       .entries(rawCommands)
       .map(([name, command]) => {
-        return [name, (...args: never[]) => command(...args)({ ...props, dispatch })]
+        return [name, (...args: never[]) => command(...args)({ ...props, dispatch: undefined })]
       })) as unknown as SingleCommands

     return {

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant