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

Why does the deployment fail?

Why does the deployment fail?

No idea - it looks like playwright download failed during the npm install part of the build. A retry might succeed. I don't think it was anything you did.

• I added the missing comment you mentioned to me.
• I removed key from the constructor as we discussed on Discord.
• Updated from the main branch and resolved conflicts.

The only problem now is that the unit test Has correct text point after removal after merge is failing and I can't figure out why.

I tried to replace the update editor with this, but it didn't work:

       editor.update(() => {
         const root = $getRoot();
        
         const paragraph = $createParagraphNode();
         const a = $createTextNode('a');
         const bb = $createTextNode('bb');
         const empty = $createTextNode('');
         const cc = $createTextNode('cc');
         const d = $createTextNode('d');
         root.append(paragraph.append(a, bb, empty, cc, d));

         setAnchorPoint({
           key: bb.getKey(),
           offset: 1,
           type: 'text',
         });

         setFocusPoint({
           key: cc.getKey(),
           offset: 1,
           type: 'text',
         });
       });

Closing this PR due to staleness! If there are new updates, please reopen the PR.

@potatowagon At the time, the Lexical team had decided to merge this PR, but for reasons of time it became outdated.

As you can see in the comments, I had found a super-performing cloning method, but then I realized that it did not clone the properties, but the references in memory, which can cause problems.

My conclusion as of today is that if you want to resume this feature, the way to do it would be with JSON.parse(JSON.stringify). This would make the DX better, but effectively the performance would be a little lower.

An intermediate compromise would be to leave manual cloning as it is now in the codebase, and for the rest of the users to use JSON.parse(JSON.stringify) when the clone method is not defined.

Food for thought. I leave you with my conclusions so that you can decide what seems best to you. I am okay with any decision.

The same thing applies to #3931

I do think we should go with this sort of approach eventually, but maybe a phased approach makes sense? Or at least something with a codemod?

I think the most important thing to do is to require all node classes to allow for a 0-argument constructor and then we can move all of the important and awkward static stuff to instances (clone, importJSON) post-construction. Passing the NodeKey on construction can be deprecated and then later removed. We could set and reset a module global on clone to seed $setNodeKey so that existingKey is never used.

maybe a phased approach makes sense? Or at least something with a codemod?

I'm not sure what you mean by phased approach. A codemod shouldn't be necessary since the idea of ​​this feature is to be backwards compatible. Only if the user does NOT define clone method (or import/exportJSON in the other PR), the generic one is used.

require all node classes to allow for a 0-argument constructor and then we can move all of the important and awkward static stuff to instances (clone, importJSON) post-construction.

That shouldn't be necessary for cloning either. A combination of
Object.create/assign and JSON.parse/stringify should be enough.

However I did think that some static methods might not be static to make it easier to extend node classes, but I don't know if it would be worth it or if performance might suffer.

This PR changes all of the constructor signatures and anyone subclassing them is going to have to change their super calls and constructor signatures accordingly to be compliant with the types.

Object.create/assign and JSON.parse/stringify are simply not sound solutions in the general case, it adds extra limitations on what properties are allowed and how the subclasses need to be implemented (private properties aren't enumerable or serialized by default for example). There are of course ways to work around that with the sort of boilerplate overrides that we do now, but I am very hesitant to propose a situation where the constructor code does not run for clones.

Object.create/assign and JSON.parse/stringify are simply not sound solutions in the general case, it adds extra limitations on what properties are allowed and how the subclasses need to be implemented (private properties aren't enumerable or serialized by default for example).

That's fine. The goal of this PR was that for non-JSON serializable nodes (which is the great minority), the user can continue using the manual method