Add async processing support to support async plugins

[mikesir87]

Resolves #680

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

Added an async option to the component. If true, runs the renderer using processor.run instead of processor.runSync (which causes an error be thrown).

[wooorm]

Thanks for the PR! Hmm, this seems to be a bit of a naïve attempt:

• there are no docs
• there is no error handling
• there are no tests that actually test async (or event sync) plugins
• I think this is more something for useCallback?
• as far as I’m aware this strategy would not work when server side rendering, could there be a way around that?

[mikesir87]

this seems to be a bit of a naïve attempt

Haha... very well might be! 😅 I tried quite a few ways to get something working that will be both backwards compatible and work for the async route.

there are no docs

My bad. I'll get on that. 😄 I did at least update the typings for the new attribute.

there is no error handling

There wasn't any previous error handling in place if the processing failed for whatever reason. Sure, in this case, it'll be an uncaught exception thrown by a Promise. Recommendations on what you'd like to see here?

there are no tests that actually test async (or event sync) plugins

I can plug in a few tests to exercise this. I did add a test that at least exercises the async rendering (why I had to add react-test-renderer).

I think this is more something for useCallback?

I'm not sure that's the case, as there's no event or callback mechanism to trigger the callback function. I actually started by running everything within a useEffect, but then that turned everything to run asynchronously (all of the unit tests then failed). So, tried to figure out a way that would work for both use cases and not violate React best practices (having conditional useEffect usage) and landed with this scenario.

as far as I’m aware this strategy would not work when server side rendering, could there be a way around that?

Based on my limited experience, it does appear that server-side rendering is unsupported for useEffect. But, it's not 100% clear if it'll work in this scenario where I'm only using useState. I don't have a SSR setup to test this out though.

My guessing is that, based on me having to use the react-test-renderer, a simple ReactDom.renderToStaticMarkup would not render the expected value.

[mikesir87]

Docs updated and added a simple async plugin to validate it both executes and the overall rendering still works.

[adamdottv]

Is this PR still alive? I'm also looking for async plugin support and wondering if this effort was ongoing. Happy to help if needed.

[wooorm]

There are still some more todos from #682 (comment) as I understand it.

[mikesir87]

As far as I'm aware (it's been a little while though), I addressed all of the concerns in the comment.

However, I did find one additional bug... if the markdown changes (eg, you have a live editor or a component is reused with new markdown), the new markdown isn't rendered. That's the challenge of trying to use the same component for both async and synchronous work. One idea is to make a completely separate component that renders asynchronously, allowing me to use useState and other hooks properly (I can't use those now because you can't have conditional hooks and using them would make all rendering async).

[ujjwalchadha8]

This is especially needed with the new react server components

Since the component is built on server side, the component itself can be async (as highlighted here) thus allowing function ReactMarkdown to be async in react-markdown.js. So at least for the server side component, the PR can be simplified to use run instead of runSync on the processor.

This also means that the server-side components cannot have 'useState', which means the PR probably needs to change to support server side components.
@mikesir87 @wooorm

[wooorm]

This is especially needed

Why?

as highlighted here

What you link is Next.js-specific documentation, about their own API for pages and layouts, which indeed can be async, I think that was already possible, and I don’t see how that relates to React?

[ChristianMurphy]

@reypm you can find the status by reading the comments directly above yours.
This is blocked by unresolved requests #682 (comment) and by unresolved discussion #682 (comment).
It will be merged when and if it is ready.

[simonpfish]

Hey @wooorm @ChristianMurphy! How can I help land this PR?

[simonpfish]

We want to use a couple of async plugins in cookbook.openai.com, and this is currently blocking us. Happy to help iterate on it to be able to merge.

[wooorm]

See the comment before yours. There’s no way to do this yet, while the React RFCs aren‘t ready.

---

@remcohaszing had an idea that could perhaps work but would need a ton of testing and probably still be a separate “experimental“ export. Use the react-server condition, which should mean that Promise<JSX.Element> is supported. Otherwise, use a hook. That might work.

[simonpfish]

Got it! I ended up just using unified directly and rehype-react