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.

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

[serializer-html][list-plugin] Investigate into serializeHtml error #2837

Merged
merged 7 commits into from
Mar 25, 2024

Conversation

dimaanj
Copy link
Collaborator

@dimaanj dimaanj commented Dec 27, 2023

Summary

Shows an error condition with serializeHtml function. It's started to happen from version 26.0.1.

The `useSlateStatic` hook must be used inside the <Slate> component's context.

Basically, this commit caused an issue: 0b8b325

Currently:

/**
* Create a React element wrapped in a Plate provider.
*/
export const createElementWithSlate = (
plateProps?: Partial<PlateProps>,
dndWrapper?: string | React.FC | React.ComponentClass
) => {
const {
editor = createPlateEditor(),
value = [],
onChange = () => {},
children,
...props
} = plateProps || {};
const plate = React.createElement(
Plate,
{
editor,
initialValue: value,
onChange,
...props,
} as PlateProps,
children
);
if (dndWrapper) {
return React.createElement(dndWrapper, null, plate);
}
return plate;
};

@dimaanj dimaanj requested review from 12joan and zbeyens December 27, 2023 14:14
@dimaanj dimaanj self-assigned this Dec 27, 2023
Copy link

changeset-bot bot commented Dec 27, 2023

🦋 Changeset detected

Latest commit: c24e4d8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@udecode/plate-serializer-html Patch
@udecode/plate Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Dec 27, 2023

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

Name Status Preview Comments Updated (UTC)
plate ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2024 6:02am

@dimaanj dimaanj changed the title [serializer-html][list-plugin] Show a failing context [serializer-html][list-plugin] Investigate into serializeHtml error Dec 27, 2023
@12joan
Copy link
Collaborator

12joan commented Dec 27, 2023

Thanks for providing this reproducing test case. Unfortunately, we can't merge this PR until the issue is resolved, since putting a failing test on main will block all other development.

I think I've figured out what's causing the issue. useStateStatic (Slate's equivalent to our useEditorRef) isn't used anywhere in Plate's code. It is, however, used in some slate-react components that Plate uses, such as Slate's text.tsx.

useSlateStatic requres a EditorContext.Provider ancestor, which should have been provided a Slate component. In Plate, this is rendered by PlateContent via PlateSlate. It seems that elementToHtml and createElementWithSlate have successfuly provided a Plate component, but have neglected to provide a PlateContent.

What I don't understand is why serializeHtml has been working at all without a PlateContent, and what it is about the example in your PR specifically that triggers it. Is it the overrideByKey option, or something inside createListPlugin, perhaps?

@dimaanj Would you be able to see if adding PlateContent to createElementWithSlate helps at all?

@zbeyens zbeyens marked this pull request as draft December 27, 2023 20:08
@dimaanj
Copy link
Collaborator Author

dimaanj commented Dec 29, 2023

Would you be able to see if adding PlateContent to createElementWithSlate helps at all?

Yes, adding it like I did it below helps to avoid errors when there are overriden elements with overrideByKey. The thing is it requires test updates. Could you please look at it @12joan ?

+  const plateContent = React.createElement(PlateContent, null, children);

  const plate = React.createElement(
    Plate,
    {
      editor,
      initialValue: value,
      onChange,
      ...props,
    } as PlateProps,
-     children
+    plateContent
  );

@12joan
Copy link
Collaborator

12joan commented Dec 29, 2023

@dimaanj Can you push that change to this branch so we can see the failures on the CI?

The fact that it only happens when there are type overrides is an interesting clue. @zbeyens Any idea what the link might be between overrideByKey and useSlateStatic?

@zbeyens
Copy link
Member

zbeyens commented Dec 29, 2023

Maybe when DefaultElement is used?

…alizeHtml-error

# Conflicts:
#	packages/serializer-html/src/utils/createElementWithSlate.ts
@dimaanj
Copy link
Collaborator Author

dimaanj commented Dec 29, 2023

@dimaanj Can you push that change to this branch so we can see the failures on the CI?

Done

@dimaanj dimaanj marked this pull request as ready for review March 24, 2024 19:31
@zbeyens zbeyens merged commit d067806 into udecode:main Mar 25, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants