Skip to content

[WIP] Deserialize/serialize#2120

Closed
trueadm wants to merge 16 commits intomainfrom
ser-des-textnode
Closed

[WIP] Deserialize/serialize#2120
trueadm wants to merge 16 commits intomainfrom
ser-des-textnode

Conversation

@trueadm
Copy link
Copy Markdown
Collaborator

@trueadm trueadm commented May 9, 2022

No description provided.

@vercel
Copy link
Copy Markdown

vercel bot commented May 9, 2022

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

Name Status Preview Updated
lexical ✅ Ready (Inspect) Visit Preview May 11, 2022 at 5:05PM (UTC)
lexical-playground ❌ Failed (Inspect) May 11, 2022 at 5:05PM (UTC)

@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 May 9, 2022
@trueadm trueadm changed the title [WIP] Deserialize/serialize TextNode [WIP] Deserialize/serialize base nodes May 9, 2022
onError: (error) => onError(error, newEditor),
readOnly: true,
theme,
serializer: new BaseSerializer<>(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of accidentally left this in here - probably should plumb it through props or something?

*
*/

import {initializeUnitTest} from '../../../../lexical/src/__tests__/utils';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be able to use absolute path here

});
}

export function $deserializeRoot<
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't mutate the existing editor state, or trigger an update. It should return a fresh new EditorState with the nodes added in.

'Serializer not defined. You can pass a new serializer via EditorConfig -> createEditor({ serializer })',
);
}
return editor.getEditorState().read(() => {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should allow to pass in an editorState as an argument too.


export function $serializeRootNode<SerializedNode>(
node: RootNode,
serializeChild: (node: LexicalNode) => SerializedNode,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should be passing this to $serializeRootNode, children should be handled by the serializer directly, as if we have deep = false then we might only want the root node and none of its children (for collab).

pendingEditorState._flushSync = true;
editor._dirtyType = FULL_RECONCILE;
editor._compositionKey = null;
editor.update(() => {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we being any update in $serializeRoot. Isn't the intention to return a fresh EditorState?

@trueadm trueadm changed the title [WIP] Deserialize/serialize base nodes [WIP] Deserialize/serialize May 10, 2022
@trueadm
Copy link
Copy Markdown
Collaborator Author

trueadm commented May 10, 2022

I also think it's important that each $deserialize and $serialize function on the nodes do not handle children, instead the Serializer class should do this.

@zurfyx
Copy link
Copy Markdown
Member

zurfyx commented May 11, 2022

I also think it's important that each $deserialize and $serialize function on the nodes do not handle children, instead the Serializer class should do this.

Shouldn't it be the responsibility of each node to understand whether to process children?

@trueadm
Copy link
Copy Markdown
Collaborator Author

trueadm commented May 11, 2022

I don't think so. Its children should be formed by the tree logic. This is important if certain children should be skipped or re-ordered etc

// No need .deserialize already does that
// $getRoot().markDirty(); // Create pendingEditorState
const serializedRootNode = json;
return createEditorState(editor, (pendingEditorState: EditorState) => {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it should be called internalCreateEditorState, so people don't use it elsewhere lol


export function createEditorState(
editor: LexicalEditor,
updateFn: (pendingEditorState: EditorState) => void,
Copy link
Copy Markdown
Collaborator Author

@trueadm trueadm May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing in pendingEditorState, you could have the expectation for it to return the RootNode instead?

createRootFn: () => RootNode

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.

4 participants