Modularize the paste process with a new INSERT_NODES_COMMAND

[GermanJablo]

Through a small refactoring, this PR splits the paste process into 2 stages: PASTE_COMMAND, and INSERT_NODES_COMMAND.

The reason for this is that I need to do some transformations on the pasted nodes, and I would end up copying a huge amount of code just to modify a couple of lines.

This allows users to intervene only in the final part of the process.

[vercel[bot]]

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

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 30, 2023 5:09am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 30, 2023 5:09am

[acywatson]

The only issue I see with this is users being confused about when to use this command, as it's pretty generic (in name).

What do you think about being more specific, like INSERT_PASTED_NODES_COMMAND?

[GermanJablo]

Curiously, I thought exactly the same thing and had written that in the initial draft of the PR. Later I deleted it because I thought that maybe that command could be useful in some other circumstance*, because in fact what it does is quite generic.

Either way, it's not something I care about very much. I leave it up to you. Do you want me to rename it?

*EDIT: It occurs to me to drag and drop text for example

[GermanJablo]

I comment to notify that I left an edit in the previous message last Friday.

---

As for me, my only reservation about this PR is to make it future proof, and not follow a specific implementation.

INSERT_NODES command does some normalization, so the current paste process is:

1. Convert clipboard to an array of nodes.
2. Normalize the array to be inserted.
3. Insert the normalized array.

My question is whether the normalization from step 2 should be part of PASTE_COMMAND or INSERT_NODES (or a third command?). On the other hand... Is this 3-stage process one implementation among other possible ones? Personally, the only thing that occurs to me is that the normalization could be done at the insertion itself or post insertion. Maybe these are not critical questions, but worth sharing. Let me know what you think.

[acywatson]

I comment to notify that I left an edit in the previous message last Friday.

As for me, my only reservation about this PR is to make it future proof, and not follow a specific implementation.

INSERT_NODES command does some normalization, so the current paste process is:

1. Convert clipboard to an array of nodes.
2. Normalize the array to be inserted.
3. Insert the normalized array.

My question is whether the normalization from step 2 should be part of PASTE_COMMAND or INSERT_NODES (or a third command?). On the other hand... Is this 3-stage process one implementation among other possible ones? Personally, the only thing that occurs to me is that the normalization could be done at the insertion itself or post insertion. Maybe these are not critical questions, but worth sharing. Let me know what you think.

This is a good question. The semantics of pasting in an editor typically couple transformation and insertion, which makes it hard to align with an external convention or example and still support this use case.

My opinion is that normalization here is built around the editors handling of insertion. We make it safe to insert at the root and rely on the insertNodes algorithm or others to handle any merging or whatever if the paste destination is somewhere else. If we're going to make this generic, I would put the override hook before normalization, so that whatever the user is going to do isn't impacted by the normalization logic. So I guess that means it needs to be part of a paste-specific INSERT_NODES command.

[GermanJablo]

If I am understanding you correctly, then I should leave it as it is now in this PR, right?

And regarding the name of the command? INSERT_PASTED_NODES_COMMAND or INSERT_NODES_COMMAND?

[acywatson]

INSERT_PASTED_NODES_COMMAND

This, I think.

[acywatson]

Also some failing Unit tests, apparently

[GermanJablo]

• name changed to INSERT_PASTED_NODES_COMMAND
• Fixed unit tests

[GermanJablo]

Something pending with this?

Is it possible that it will be in the next release?

[nerdess]

@GermanJablo I need to also cleanup the nodes after a user has pasted content (e.g. from MS Word) into Lexical. Your PR looks helpful for that!

Can you maybe post some sample code how you utilized your PR and achieved a transformation on your pasted nodes? I am struggeling to intercept into the pasting-process.

[GermanJablo]

I'm working right now on fixing the tests. As some entire suites passed I thought those who failed were flaky.

@nerdess you have tried the new command on this PR, right?

[nerdess]

@GermanJablo Not yet, I was first trying to get the already existing PASTE_COMMAND to work but it does not get triggered upon pasting and I am wondering why. So I thought I could peek at your code and get some inspiration.

useEffect(() => {
      return mergeRegister(
            editor.registerCommand(
                PASTE_COMMAND,
                (payload) => {
                    console.log('paste', payload);  //This never happens!!! :(
                    return true;
                },
                COMMAND_PRIORITY_EDITOR,
              ),
       );     
}, [editor]);

[acywatson]

@GermanJablo Not yet, I was first trying to get the already existing PASTE_COMMAND to work but it does not get triggered upon pasting and I am wondering why. So I thought I could peek at your code and get some inspiration.

useEffect(() => {
      return mergeRegister(
            editor.registerCommand(
                PASTE_COMMAND,
                (payload) => {
                    console.log('paste', payload);  //This never happens!!! :(
                    return true;
                },
                COMMAND_PRIORITY_EDITOR,
              ),
       );   
}, [editor]);

Did you try a higher priority? COMMAND_PRIORITY_EDITOR is what Lexical uses internally, generally, so if there's another listener at that priority it could run first and stop propagation to your listener.

[acywatson]

I'm working right now on fixing the tests. As some entire suites passed I thought those who failed were flaky.

Yea, not an unreasonable assumption to make, given condition of the CI right now.

[nerdess]

@acywatson ok that is some new learning....didn't know that listeners on the same priority can cancel the remaining ones...the command works now when using COMMAND_PRIORITY_HIGH

@GermanJablo wanted to use your fork instead of the lexical repo but somehow it always turns out

"@lexical/monorepo": "GermanJablo/lexical",

in package.json?!

I expect

"lexical": "GermanJablo/lexical"

Might just wait for the PR to be merged...

[GermanJablo]

@nerdess The way to test the code is to checkout the PR. If you want to incorporate the changes to your project, yes, you should wait for it to merge.

Trying to fix the tests in this PR, I ended up redoing the insertNodes method in another PR.

[GermanJablo]

I found a better way to achieve this.

Even with a command to insert nodes the user would have to modify a piece of code that is very large and complex like insertNodes.

https://github.com/facebook/lexical/pull/5201 makes this PR obsolete, as it allows you to "modify" pasted nodes simply by overriding the insertRangeAfter method.