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

Render output with rehype-react #213

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Render output with rehype-react #213

merged 1 commit into from
Oct 30, 2023

Conversation

eatyourgreens
Copy link
Contributor

Remove dangerouslySetInnerHTML. Render the HTML output with rehype-react.

Remove `dangerouslySetInnerHTML`. Render the HTML output with `rehype-react`.
@eatyourgreens
Copy link
Contributor Author

This works in PFE. 👍

@eatyourgreens eatyourgreens merged commit 8807237 into master Oct 30, 2023
3 checks passed
@eatyourgreens eatyourgreens deleted the rehype branch October 30, 2023 16:26
@goplayoutside3
Copy link
Contributor

goplayoutside3 commented Oct 30, 2023

For documentation, what was the reason behind adding rehype here?


Linking some related comments:
Comment in FEM PR 5253 asking about dangerouslySetInnerHTML: zooniverse/front-end-monorepo#5352 (review)

Comment in FEM PR 5253 explaining that utils.getHTML(markdown) now sanitises the output HTML string with DOMPurify, before it's passed to Rehype in FEM's Markdownz component: zooniverse/front-end-monorepo#5352 (comment)

@eatyourgreens
Copy link
Contributor Author

For documentation, what was the reason behind adding rehype here?

To replace dangerouslySetInnerHTML with React children here:

return createElement(tag, {
className: `markdown ${className}`,
children: parsedHTML,
ref: (element) => { this.root = element; }
});

The Markdown component now renders a React component tree, rather than HTML strings.

@eatyourgreens
Copy link
Contributor Author

I've added an example in #216, showing how you can safely render markdown as JSX.

@goplayoutside3
Copy link
Contributor

Cool, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants