Skip to content

Fix list deserialization#4030

Merged
acywatson merged 5 commits intomainfrom
list-deser
Mar 7, 2023
Merged

Fix list deserialization#4030
acywatson merged 5 commits intomainfrom
list-deser

Conversation

@acywatson
Copy link
Copy Markdown
Contributor

@acywatson acywatson commented Mar 4, 2023

This one was a bit gnarly and probably requires a deeper fix at some point. We've had reports of people encountering issues when deserializing lists, and it all goes back to this issue: #3227

What we're doing here is basically trying to update Lexical nodes during the reconciliation phase, which typically works fine, but encounters some issues in a few cases. One of those is when reconciliation runs asynchronously, which seems to be what's happening in the case of state being initialized from a string via LexicalComposer. Basically, we "freeze" the node map after the update cycle to ensure that state can't be further updated outside of that cycle. In this case, we were freezing the map, then createDOM was running, calling these list item update functions, which tried to update the nodes. There are several interrelated issues here, including the underlying structure/function of lists, which we will ultimately probably change (as discussed elsewhere).

The fix I'm proposing here is to introduce a method to nodes that allows them to declare a transform on themselves, rather than registering it through the register API. This allows us to keep this logic encapsulated at the node level, rather than moving it to the plugin, and it should otherwise function the same.

fixes #3975 and #3227

@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 Mar 4, 2023
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 4, 2023

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

Name Status Preview Comments Updated
lexical ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 7, 2023 at 2:22PM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 7, 2023 at 2:22PM (UTC)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/lexical/dist/Lexical.js 26.09 KB (+0.03% 🔺) 522 ms (+0.03% 🔺) 60 ms (+18.41% 🔺) 581 ms
packages/lexical-rich-text/dist/LexicalRichText.js 36.91 KB (+0.06% 🔺) 739 ms (+0.06% 🔺) 45 ms (-4.93% 🔽) 783 ms
packages/lexical-plain-text/dist/LexicalPlainText.js 36.88 KB (+0.06% 🔺) 738 ms (+0.06% 🔺) 46 ms (-7.52% 🔽) 783 ms

@moy2010
Copy link
Copy Markdown
Contributor

moy2010 commented Mar 4, 2023

One deserialization error that I've had before is caused by the fact that markdown's shortcut's regex for the numeric list matches on any number, so it's possible to create numeric lists that do not start from 1..

@acywatson
Copy link
Copy Markdown
Contributor Author

One deserialization error that I've had before is caused by the fact that markdown's shortcut's regex for the numeric list matches on any number, so it's possible to create numeric lists that do not start from 1..

Oh weird. I guess starting a list at a number other than 1 should be possible. Might need to look into that separately.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: List/ListItem Nodes From DOM results in invalid editor state

4 participants