Description

One of the biggest pain points and error-prone parts of extending lexical is the amount of boilerplate involved in subclassing nodes. In particular the worst of it is the static importJSON method that must be implemented. We can eventually make the boilerplate go away with some targeted refactoring.

The majority of this PR implements updateFromJSON which is the importJSON analog to the afterCloneFrom for clone from #6505. Basically the key concept here is to move all of the logic, other than the constructor itself (with as many defaults as possible), into an instance method, so that super can be called to inherit all of the base class logic.

More documentation has been added related to serialization best practices, including a recommendation not to use the version property as it does not compose well and is generally just ignored and/or redundant.

Before:

class BaseNode {
  static importJSON(serializedNode: SerializedBaseNode): BaseNode {
    const node = $createBaseNode();
    /* … a whole bunch of boilerplate that must be copied and pasted in subclasses */
    return node;
  }
}

class ExtendedNode extends BaseNode {
  static importJSON(serializedNode: SerializedExtendedNode): ExtendedNode {
    const node = $createExtendedNode();
    /* … a whole bunch of boilerplate that is copied from BaseClass */
    /* … some extra stuff specific to SerializedExtendedNode */
    return node;
  }
}

After:

class BaseNode {
  // We can eventually eliminate this boilerplate altogether, but makes sense in the short term
  static importJSON(serializedNode: SerializedBaseNode): BaseNode {
    return $createBaseNode().updateFromJSON(serializedNode);
  }

  updateFromJSON(serializedNode: Omit<SerializedBaseNode, 'type' | 'version'>): this {
    // for the true base cases of TextNode and ElementNode this will not have a super call
    const node = super.updateFromJSON(serializedNode);
    /* … the previous boilerplate, that can now be inherited */
    return node;
  }
}

class ExtendedNode extends BaseNode {
  // We can eventually eliminate this boilerplate altogether, but makes sense in the short term
  static importJSON(serializedNode: SerializedExtendedNode): ExtendedNode {
    return $createExtendedNode().updateFromJSON(serializedNode);
  }

  // this is now *optional* and can be implemented when extra setters are needed
  updateFromJSON(serializedNode: Omit<SerializedExtendedNode, 'type' | 'version'>): this {
    const node = super.updateFromJSON(serializedNode);
    /* … some extra stuff specific to SerializedExtendedNode */
    return node;
  }
}

The motivation for this was to move textFormat and textStyle to ElementNode, which to do properly would require also updating serialization everywhere, so it made sense to eliminate the boilerplate at the same time.

Note that the types here are not quite sound and are probably not possible to type in Flow, for the same class of reason that updateDOM and afterCloneFrom are also not sound. When a subclass implements this method with a more specific type, it's no longer sound to upcast it to the base class and call the method with the schema intended for the base class. I'm sure it would be possible to come up with a scheme that is sound, but I think it would require a large DX trade-off one way or another (such as using a new method name for each class like updateFromTextNodeJSON, moving it to a static method or exported function that must be called fully qualified, or forcing the updateFromJSON implementations to parse the JSON from something like Record<unknown, unknown> - which would be much safer but also very tedious with a runtime penalty). See #6998 for more information about this and some ideas for runtime checks.

Closes #6949
Closes #6476
Closes #5708 (although it does not implement a perfect solution to persist styles at arbitrary positions, it's better than status quo)

Might get closer to fixing #6583

Related: #3931

Upgrade Notes

This change adds optional textFormat and textStyle properties to SerializedElementNode. If you have existing classes with those properties it could create a namespace clash that you will have to resolve one way or another.

TextNode and ElementNode subclasses should be updated to call the updateFromJSON(serializedNode) method from their static importJSON methods. If they don't, they won't support this new functionality, and will have to continue copy and pasting the super implementation of importJSON for correct behavior if the base class ever changes in the future.

You should consider dropping usage of the version field.

Test plan

• Existing unit and e2e tests all pass (serialization tests and tests of the exact instance representation were modified)
• New e2e test for ListItemNode behavior
• Other tests and code were refactored to use this new facility, so it's well covered by everything else too

Before

Styles were not preserved when creating a new ListItem

After

Styles are preserved when creating a new ListItem (including e2e test)