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

Theme: Children interface is now ReactNode, allows guard() and render() to accept JSX #971

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

robbieaverill
Copy link
Contributor

@robbieaverill robbieaverill commented Sep 18, 2024

Description

This fixes a type incompatibility in theme components where using guard() and returning a React or DOM element would not pass type checking. The Children interface now extends ReactNode for backwards compatibility and is marked as deprecated. Use ReactNode directly in future.

Duplicated markdown utility functions in the theme are now deprecated in favour of their equivalents in the plugin package. Edit: plugin methods remain the same, they look similar but serve a slightly different purpose.

Motivation and Context

Refer to theme: ParamsItem/index.tsx:

const renderDeprecated = guard(deprecated, () => (
  <span className="openapi-schema__deprecated">deprecated</span>
));

The return value of the callback in this example was invalid, as a JSX.Element is not a string, etc. I've replaced the Children type with ReactNode, which supports React elements/JSX.Element, as well as all the other values that were there (string, undefined, number, iterable variants of it, etc).

How Has This Been Tested?

Type checked locally.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Deprecation of existing APIs (requires minor release)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

…ated in favour of plugin methods

This fixes a type incompatibility where using `guard()` and returning
a React or DOM element would not pass type checking. The Children
interface now extends ReactNode for backwards compatibility and is
marked as depreacated. Use ReactNode directly in future.

Duplicated markdown utility functions in the theme are now deprecated
in favour of their equivalents in the plugin package.
Copy link

github-actions bot commented Sep 18, 2024

Visit the preview URL for this PR (updated for commit 2fcfe32):

https://docusaurus-openapi-36b86--pr971-vfe03kyz.web.app

(expires Thu, 24 Oct 2024 13:05:31 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: bf293780ee827f578864d92193b8c2866acd459f

@sserrata
Copy link
Member

sserrata commented Sep 23, 2024

Hi @robbieaverill, some descriptions don't appear to be rendering in the preview:

Screenshot 2024-09-23 at 11 17 16 AM

if (Array.isArray(children)) {
const filteredChildren = children.filter((c) => c !== undefined);
return filteredChildren
.map((i: any) => (Array.isArray(i) ? i.join("") : i))
.join("");
}
return children ?? "";
return `${children ?? ""}`;
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be what results in description not rendering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some playing around. The plugin's render() method is used to create HTML strings, e.g. <p>foo</p> that are used to create the MDX files, so it doesn't actually support rendering JSX or React elements directly.

The theme does, as in my example of using guard() in a React template context.

I've pushed a new commit which removes all of the changes to the plugin and most of the deprecations in the theme, and changes the theme's guard() and render() methods to handle ReactNode. How do you feel about that?

image

@robbieaverill robbieaverill changed the title Children interface is now ReactNode, theme utility methods are deprecated in favour of plugin methods Theme: Children interface is now ReactNode, allows guard() and render() to accept JSX Sep 23, 2024
@sserrata sserrata merged commit 296475b into PaloAltoNetworks:main Sep 24, 2024
15 checks passed
@robbieaverill robbieaverill deleted the children-reactnode branch September 24, 2024 19:57
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.

2 participants