Skip to content

[Breaking Change][lexical] Feature: Add updateFromJSON and move more textFormat/textStyle to ElementNode#6970

Merged
etrepum merged 10 commits intofacebook:mainfrom
etrepum:updateFromJSON-element-node-text-format-style
Jan 1, 2025
Merged

[Breaking Change][lexical] Feature: Add updateFromJSON and move more textFormat/textStyle to ElementNode#6970
etrepum merged 10 commits intofacebook:mainfrom
etrepum:updateFromJSON-element-node-text-format-style

Conversation

@etrepum
Copy link
Copy Markdown
Collaborator

@etrepum etrepum commented Dec 16, 2024

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)

@vercel
Copy link
Copy Markdown

vercel bot commented Dec 16, 2024

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 Dec 26, 2024 4:54am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 26, 2024 4:54am

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 16, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 16, 2024

size-limit report 📦

Path Size
lexical - cjs 31.21 KB (0%)
lexical - esm 31.1 KB (0%)
@lexical/rich-text - cjs 40.31 KB (0%)
@lexical/rich-text - esm 33.01 KB (0%)
@lexical/plain-text - cjs 38.85 KB (0%)
@lexical/plain-text - esm 30.22 KB (0%)
@lexical/react - cjs 42.13 KB (0%)
@lexical/react - esm 34.26 KB (0%)

@zurfyx
Copy link
Copy Markdown
Member

zurfyx commented Dec 17, 2024

Thanks Bob, the static import has indeed been a problem for a very long time, we had this similar issue for clone where we hardcoded some properties just to make it easier for the common cases, I believe you fixed this one.

I like the solution you've presented and how it works with types!

I'm not so sure about the moving textFormat and textStyle, is this really what need? When is format relevant for an arbitrary element?

On a separate note, and beyond this PR, I'm somewhat concerned about the growing number of methods in Nodes. While it's easy to extend and for the most part they're self-explanatory it takes a fair amount of time to configure a new Node and understand what the relevant methods are. I'd be keen take this offline and discuss potential alternatives, including higher-level abstractions and codegen.

@etrepum
Copy link
Copy Markdown
Collaborator Author

etrepum commented Dec 17, 2024

The textFormat and textStyle is only relevant to an element because we don't have another reliable way to preserve it when there isn't a TextNode present. Previously we were doing this only for ParagraphNode, but it probably makes sense for any element that can contain a TextNode (such as ListItemNode in this particular use case). It's not used directly on the element for any purpose, it's really just storage for the selection. Possibly a future direction might be to phase it out and insert some kind of format preservation node (a special empty TextNode that doesn't get normalized away or maybe even rendered, for example).

With regard to methods, the only real reason we need to add methods here is backwards compatibility. If we didn't have that constraint we could get rid of static clone and static importFromJSON and replace them with only afterCloneFrom and updateFromJSON which are both optional and only needed if you are adding state to the node. exportJSON could also be optional if we had the base classes use type: this.getType(). Implementing the property overrides for the collection of afterCloneFrom, updateFromJSON, exportJSON (and all of the property getters and setters) could be done with code gen at runtime (e.g. with something like a decorator) or compile time possibly with the assistance of a fourth method that just listed the properties that need to be carried around (but of course overriding could also customize this).

I think the next step here is to allow classes to drop static clone. The strategy here would be to deprecate the nodeKey argument and allow $setNodeKey to have some module global state for what the next nodeKey is that is set and change the implementation of $cloneWithProperties accordingly

  // before
  const constructor = latestNode.constructor;
  const mutableNode = constructor.clone(latestNode) as T;
  mutableNode.afterCloneFrom(latestNode);
  // after
  const constructor = latestNode.constructor;
  let mutableNode: T;
  if (Object.hasOwn(constructor, 'clone')) {
    // backwards compatibility
    mutableNode = constructor.clone() as T;
  } else {
    $setNextNodeKey(latestNode.getKey());
    // this does require all constructors to have a 0-arg form and a correct afterCloneFrom
    mutableNode = new constructor() as T;
    // if __DEV__ make sure the key was consumed by $setNodeKey(this, key);
  }
  mutableNode.afterCloneFrom(latestNode);

Instead of checking that these static methods exist we can instead have a lint that looks for extra properties on a node and if they exist then expect an afterCloneFrom, updateFromJSON, and exportJSON to be defined.

@etrepum
Copy link
Copy Markdown
Collaborator Author

etrepum commented Dec 23, 2024

The exportJSON refactor is implemented in #6983

@etrepum etrepum added this pull request to the merge queue Jan 1, 2025
Merged via the queue into facebook:main with commit 7c21d4f Jan 1, 2025
@etrepum etrepum deleted the updateFromJSON-element-node-text-format-style branch January 5, 2025 17:07
GermanJablo added a commit to GermanJablo/lexical that referenced this pull request Jan 30, 2025
commit a62a1a6
Author: James Fitzsimmons <119275535+james-atticus@users.noreply.github.com>
Date:   Thu Jan 30 19:13:35 2025 +1100

    [lexical-mark] Feature: include inline decorator nodes in marks (facebook#7086)

commit 881c7fe
Author: Bob Ippolito <bob@redivi.com>
Date:   Thu Jan 30 00:13:00 2025 -0800

    [lexical-utils] Fix: Modify $reverseDfs to be a right-to-left variant of $dfs (facebook#7112)

commit ce2bb45
Author: Nigel Gutzmann <nigelgutzmann@gmail.com>
Date:   Wed Jan 29 14:41:12 2025 -0800

    [lexical-utils] Feature: add reverse dfs iterator (facebook#7107)

commit 3a140d2
Author: mohammed shaheer kp <72137242+mshaheerz@users.noreply.github.com>
Date:   Tue Jan 28 06:19:45 2025 +0530

    [lexical-playground] Bug Fix: Ensure Delete Node handles all node types (facebook#7096)

    Co-authored-by: shaheerkpzaigo <mohammedshaheer@zaigoinfotech.com>

commit 8e2ede2
Author: Adam Pugh <docadam@meta.com>
Date:   Mon Jan 27 18:49:38 2025 -0600

    Listeners Lexical: 3 updates to spelling and grammar - Update listeners.md  (facebook#7100)

commit 9fcc494
Author: Adam Pugh <docadam@meta.com>
Date:   Mon Jan 27 18:49:34 2025 -0600

    Lexical Docs: 2 updates to spelling README.md (facebook#7102)

commit 946a6df
Author: Adam Pugh <docadam@meta.com>
Date:   Mon Jan 27 18:49:29 2025 -0600

    Selection | Lexical: 1 Spelling Update Update selection.md (facebook#7103)

commit ce93ea6
Author: Adam Pugh <docadam@meta.com>
Date:   Mon Jan 27 18:49:25 2025 -0600

    Creating a React Plugin: 1 Grammar Update - Update create_plugin.md (facebook#7104)

commit ed29d89
Author: Adam Pugh <docadam@meta.com>
Date:   Mon Jan 27 18:49:21 2025 -0600

    Working with DOM Events: 2 Spelling and Grammar Updates Update dom-ev… (facebook#7105)

commit 212b70f
Author: James Fitzsimmons <119275535+james-atticus@users.noreply.github.com>
Date:   Mon Jan 27 08:48:09 2025 +1100

    [lexical-yjs] Bug Fix: handle text node being split by Yjs redo (facebook#7098)

commit 6a98a47
Author: Torleif Berger <torleif@outlook.com>
Date:   Fri Jan 24 21:46:45 2025 +0100

    [lexical-react] Bug Fix: Import `JSX` type from React to prevent "Cannot find namespace 'JSX'"-error when type-checking with React 19 (facebook#7080)

commit f8e5968
Author: Tetsuya <62472294+EruditionTu@users.noreply.github.com>
Date:   Sat Jan 25 04:06:57 2025 +0800

    [lexical] Chore: Rename variable and add comments for Safari compositing workaround (facebook#7092)

commit 81c9ab6
Author: Mateo Vuković <195247756+hellovuki@users.noreply.github.com>
Date:   Fri Jan 24 18:44:05 2025 +0100

    Fix: Use already defined RegisteredNodes type (facebook#7085)

commit 63958a2
Author: Sherry <potatowagon@meta.com>
Date:   Tue Jan 21 18:15:21 2025 +0800

    [playground] Bug fix: prevent growing whitespaces in markdown table toggle (facebook#7041)

    Co-authored-by: Bob Ippolito <bob@redivi.com>

commit d9f9924
Author: Sherry <potatowagon@meta.com>
Date:   Tue Jan 21 14:58:08 2025 +0800

    Unrevert [Breaking Change][lexical] Bug Fix: Commit updates on editor.setRootElement(null) facebook#7023 (facebook#7068)

commit 92fa0a3
Author: mohammed shaheer kp <72137242+mshaheerz@users.noreply.github.com>
Date:   Tue Jan 21 06:23:24 2025 +0530

    [lexical-playground] plugins TableOfContent Scroll smooth behaviour A… (facebook#7069)

commit 2e4a63e
Author: Ivaylo Pavlov <ivailo90@gmail.com>
Date:   Mon Jan 20 02:37:34 2025 +0000

    [lexical-playground] Fix Columns Layout Item Overflow (facebook#7066)

commit d319b07
Author: Bob Ippolito <bob@redivi.com>
Date:   Sun Jan 19 14:45:41 2025 -0800

    Change fork modules to use production only when NODE_ENV explicitly set to production (facebook#7065)

commit 46c9c2f
Author: CityHunter <62472294+EruditionTu@users.noreply.github.com>
Date:   Sat Jan 18 13:00:38 2025 +0800

    [lexical] Bug Fix: In the Safari browser, during the compositing event process, the delete key exhibits unexpected behavior. (facebook#7061)

    Co-authored-by: 涂博闻 <tubowen@moonshot.cn>

commit 92a1cd7
Author: Violet Rosenzweig <rosenzweig.violet@gmail.com>
Date:   Thu Jan 16 18:44:11 2025 -0500

    docs: Change "here" link to more descriptive text (facebook#7058)

commit f6377a3
Author: Aman Harwara <amanharwara@protonmail.com>
Date:   Fri Jan 17 02:08:17 2025 +0530

    [lexical-table] Bug Fix: Prevent error if pasted table has empty row (facebook#7057)

commit 0835029
Author: Aman Harwara <amanharwara@protonmail.com>
Date:   Fri Jan 17 00:18:08 2025 +0530

    [lexical-list] Bug Fix: Prevent error when calling formatList when selection is at root (facebook#6994)

commit 940435d
Author: Brayden <1311325+redstar504@users.noreply.github.com>
Date:   Wed Jan 15 16:10:01 2025 -0800

    fix: iOS Autocorrect strips formatting by reporting wrong dataType (facebook#5789)

    Co-authored-by: Bob Ippolito <bob@redivi.com>

commit 136a565
Author: Aman Harwara <amanharwara@protonmail.com>
Date:   Thu Jan 16 04:48:32 2025 +0530

    [lexical-yjs] Feature: Allow passing in custom `syncCursorPositions` function to collab hook (facebook#7053)

commit 415c576
Author: Maksim Horbachevsky <fantactuka@gmail.com>
Date:   Wed Jan 15 18:18:03 2025 -0500

    fix: triple click around inline elements (links) (facebook#7055)

commit a3ef4f3
Author: Ivaylo Pavlov <ivailo90@gmail.com>
Date:   Wed Jan 15 23:15:39 2025 +0000

    [lexical-table] Support table alignment (facebook#7044)

commit 29d733c
Author: Sherry <potatowagon@meta.com>
Date:   Wed Jan 15 21:50:07 2025 +0800

    Revert [Breaking Change][lexical] Bug Fix: Commit updates on editorSetRootElement(null) (facebook#7023) (facebook#7052)

commit 65ce66a
Author: Bob Ippolito <bob@redivi.com>
Date:   Tue Jan 14 14:57:54 2025 -0800

    [lexical] Bug Fix: Normalize selection after applyDOMRange to account for Firefox differences (facebook#7050)

commit bbc07af
Author: Bob Ippolito <bob@redivi.com>
Date:   Tue Jan 14 08:55:46 2025 -0800

    [*] Bug Fix: Use GITHUB_OUTPUT instead of GITHUB_ENV for size-limit action (facebook#7051)

commit c8f27ed
Author: Bob Ippolito <bob@redivi.com>
Date:   Tue Jan 14 06:36:13 2025 -0800

    [Breaking Change][*] Chore: Use terser for optimizing cjs prod build (facebook#7047)

commit 8bd22d5
Author: Bob Ippolito <bob@redivi.com>
Date:   Mon Jan 13 07:09:31 2025 -0800

    [lexical] Bug Fix: Handle MutationObserver/input event re-ordering when using contentEditable inside of an iframe (facebook#7045)

commit 930629c
Author: Ivaylo Pavlov <ivailo90@gmail.com>
Date:   Sat Jan 11 06:03:30 2025 +0000

    Clean up nested editor update (facebook#7039)

commit bd874a3
Author: Bob Ippolito <bob@redivi.com>
Date:   Fri Jan 10 15:23:54 2025 -0800

    [Breaking Change][lexical][lexical-selection][lexical-list] Bug Fix: Fix infinite loop when splitting invalid ListItemNode (facebook#7037)

commit 541fa43
Author: Bob Ippolito <bob@redivi.com>
Date:   Thu Jan 9 12:42:23 2025 -0800

    v0.23.1 (facebook#7035)

    Co-authored-by: Lexical GitHub Actions Bot <>

commit d7abafd
Author: Bob Ippolito <bob@redivi.com>
Date:   Thu Jan 9 08:33:12 2025 -0800

    [Breaking Change][lexical] Bug Fix: Commit updates on editor.setRootElement(null) (facebook#7023)

commit 6add515
Author: Bob Ippolito <bob@redivi.com>
Date:   Wed Jan 8 17:27:15 2025 -0800

    [lexical] Fix TabNode deserialization regression  (facebook#7031)

commit 33e3677
Author: Maksim Horbachevsky <fantactuka@gmail.com>
Date:   Wed Jan 8 14:59:03 2025 -0500

    [lexical-react] Feature: Merge TabIndentionPlugin and ListMaxIndentLevelPlugin plugins (facebook#7018)

commit 7de86e4
Author: James Fitzsimmons <119275535+james-atticus@users.noreply.github.com>
Date:   Wed Jan 8 09:45:25 2025 +1100

    [lexical-mark] Bug Fix: reverse ternary in MarkNode.addID (facebook#7020)

commit 7961130
Author: Bob Ippolito <bob@redivi.com>
Date:   Sun Jan 5 13:55:25 2025 -0800

    v0.23.0 (facebook#7017)

    Co-authored-by: Lexical GitHub Actions Bot <>

commit 2b4252d
Author: Aman Harwara <amanharwara@protonmail.com>
Date:   Sat Jan 4 11:31:19 2025 +0530

    [lexical-yjs] Feature: Expose function to get anchor and focus nodes for given user awareness state (facebook#6942)

commit 8100d6d
Author: Ivaylo Pavlov <ivailo90@gmail.com>
Date:   Sat Jan 4 01:12:04 2025 +0000

    [lexical-playground] Fix table hover actions button position (facebook#7011)

commit bd1ef2a
Author: Bob Ippolito <bob@redivi.com>
Date:   Fri Jan 3 14:25:31 2025 -0800

    [lexical] Bug Fix: Fix registerNodeTransform regression introduced in facebook#6894 (facebook#7016)

commit 85c08b6
Author: Christian Grøngaard <christian@groengaard.dk>
Date:   Thu Jan 2 00:20:20 2025 +0100

    [lexical-playground] Refactor: switch headings test file names (facebook#7008)

commit 7c21d4f
Author: Bob Ippolito <bob@redivi.com>
Date:   Wed Jan 1 12:48:12 2025 -0800

    [Breaking Change][lexical] Feature: Add updateFromJSON and move more textFormat/textStyle to ElementNode (facebook#6970)

commit aaa9009
Author: Bob Ippolito <bob@redivi.com>
Date:   Wed Jan 1 07:50:39 2025 -0800

    [lexical] Bug Fix: Fix getNodes over-selection (facebook#7006)

commit 803391d
Author: Sherry <potatowagon@meta.com>
Date:   Tue Dec 31 11:26:17 2024 +0800

    [__test__] npm upgrade astro (facebook#7001)

commit 684352b
Author: Christian Grøngaard <christian@groengaard.dk>
Date:   Mon Dec 30 05:12:45 2024 +0100

    Documentation: Fix typo "nest nest"->"nest" in README.md (facebook#7000)

    Co-authored-by: Bob Ippolito <bob@redivi.com>

commit 27b75cc
Author: Sherry <potatowagon@meta.com>
Date:   Fri Dec 27 11:06:29 2024 +0800

    [__tests__] npm upgrade next (facebook#6996)

commit 05ddbcc
Author: Simon <bauchet.simon@gmail.com>
Date:   Thu Dec 26 03:37:50 2024 +0100

    [lexical] Bug Fix: Flow is missing some variables and functions (facebook#6977)

commit e79c946
Author: Sherry <potatowagon@meta.com>
Date:   Tue Dec 24 09:54:46 2024 +0800

    v0.22.0 (facebook#6993)

    Co-authored-by: Lexical GitHub Actions Bot <>

commit c415f7a
Author: Sam Zhou <sam@developersam.com>
Date:   Mon Dec 23 10:31:36 2024 -0800

    [lexical-react] Refactor: Replace `React$MixedElement` and `React$Node` with `React.MixedElement` and `React.Node` (facebook#6984)

commit c844a4d
Author: Sherry <potatowagon@meta.com>
Date:   Tue Dec 24 02:30:52 2024 +0800

    [lexical] Fix flow error: change this to any (facebook#6992)

commit 6190033
Author: Germán Jabloñski <43938777+GermanJablo@users.noreply.github.com>
Date:   Mon Dec 23 05:19:27 2024 -0300

    Refactor: exportJSON (facebook#6983)

commit e0dafb8
Author: Germán Jabloñski <43938777+GermanJablo@users.noreply.github.com>
Date:   Sat Dec 21 13:59:01 2024 -0300

    feature: expose forEachSelectedTextNode (facebook#6981)

    Co-authored-by: Bob Ippolito <bob@redivi.com>

commit 23715f5
Author: Alex <UlopLT@gmail.com>
Date:   Fri Dec 20 18:23:27 2024 +0300

    [lexical][lexical-table] Bug fix: TablePlugin:  - check is current selection in target table node (facebook#6979)

    Co-authored-by: alazarev <alazarev@megaputer.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Preserve original format after indenting list item Bug: Selection formatting/styling is lost when you indent a list ElementNode format

3 participants