Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exportJSON to ArtificialNode #6107

Closed
wants to merge 1 commit into from
Closed

Add exportJSON to ArtificialNode #6107

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/lexical/src/nodes/ArtificialNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,8 @@ export class ArtificialNode__DO_NOT_USE extends ElementNode {
const dom = document.createElement('div');
return dom;
}

exportJSON() {
return super.exportJSON();
Copy link
Member

Choose a reason for hiding this comment

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

Let's make it throw an invariant instead please. This Node should not exist and should never land on any database.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a commit in #6048 that just ignores the warnings for these nodes, which is another approach that would preserve expected runtime errors without sacrificing DX 8cd0664

Copy link
Contributor

Choose a reason for hiding this comment

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

im not sure if changing to invariant would continue to raise errors. the browser warnings was reported to break the serialisation of the editor state on KP's usecase (T189214869), hence the goal of the PR to silence the warnings.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a commit in #6048 that just ignores the warnings for these nodes, which is another approach that would preserve expected runtime errors without sacrificing DX 8cd0664

thanks for this, lgtm!

}
Copy link
Contributor

Choose a reason for hiding this comment

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

#5966 reports that there are a couple more unimplemented methods that are triggering console warnings. if u have bandwidth could u help add those in? if not ill be happy to add those in after this PR is merged

}
Loading