-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add support for plugins #58
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
base: main
Are you sure you want to change the base?
Conversation
|
@szymonograbek Thank you! I will take a look these days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm excited by the support for plugins so I had a look at that part of the PR to try and fasten its inclusion in the lib. Thanks for doing it!
As I commented, I think considering remark-parse like an optional plugin leads to problems so I would recommend forcing its usage
Also, this comment in the original issue even recommends to drop suggested plugins and let users use whichever plugin they need. It could reduce the bundle size to avoid importing remark-gfm in all cases but I wonder if it wouldn't be misleading to say the lib support GFM (because it includes the appropriate renderers) while still requiring users to pass the plugin manually. Wdyt @szymonograbek and @imwithye ?
Finally, I think adding a mermaid example isn't really linked to adding support for custom plugins, though originating from the same discussion, so it would be cleaner to put it in a separate PR to avoid bloating this one and ease the review and acceptance or not of each subject (mermaid example represents more than 98% of the changed lines)
| }: MarkdownProps) => { | ||
| const tree = useMemo(() => parser.parse(markdown), [markdown]); | ||
| const processor = useMemo(() => { | ||
| return unified().use(remarkPlugins); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
| return unified().use(remarkPlugins); | |
| return unified().use(remarkParse).use(remarkPlugins); |
| import { Styles, mergeStyles } from "./themes/themes"; | ||
|
|
||
| const parser = unified().use(remarkParse).use(remarkGfm); | ||
| const defaultRemarkPlugins: PluggableList = [remarkParse, remarkGfm]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think remarkParse shouldn't be included in the default plugins and the lib should rather use it by default because
- users can pass an empty array, which would lead to a runtime error (
TypeError: Cannot "processSync" without "Parser") remarkParseis the official remark parser so users will always want to use it when customizing the plugins list so using it by default would reduce the boilerplate
This would give
| const defaultRemarkPlugins: PluggableList = [remarkParse, remarkGfm]; | |
| const defaultRemarkPlugins: PluggableList = [remarkGfm]; |
I've though about it and I agree, we should keep
Sure, I've removed it from this PR |
|
Thank you for your PR! I will review it this week. Overall, it looks good to me! |
📝 Description
This PR adds support to pass plugins to the processor, so we can use i.e.
rehypeParseandrehypeRemarkto parse HTML to markdown. I've added temporal samples as typescript files to test this locally, if this PR will be accepted then I'll move them to the rest of markdown files.📌 Related Issue
Link: #43
📷 Screenshots
Parsed HTML:

✅ Checklist
feat:,fix:,chore:).md) used for testingpnpm run lint:fixto fix formatting and lint errorspnpm i && cd example && pnpm i && pnpm run ios)