Some plugins require async run

mikesir87 commented 2 years ago

Initial checklist

[X] I read the support docs
[X] I read the contributing guide
[X] I agree to follow the code of conduct
[X] I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

8.0.2

Link to runnable example

https://codesandbox.io/s/nice-austin-wrug0t

Steps to reproduce

Install remark-code-frontmatter
Install remark-code-extra
Configure the component to use both plugins
See error

Expected behavior

The render still works.

Actual behavior

When using the remark-code-frontmatter and remark-code-extra plugins, I'm getting the following error:

index.js:566 Uncaught (in promise) Error: `runSync` finished async. Use `run` instead
    at assertDone (index.js:566)
    at Function.runSync (index.js:352)
    at ReactMarkdown (react-markdown.js:107)
    at renderWithHooks (react-dom.development.js:14985)
    at mountIndeterminateComponent (react-dom.development.js:17811)
    at beginWork (react-dom.development.js:19049)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994)
    at invokeGuardedCallback (react-dom.development.js:4056)
    at beginWork$1 (react-dom.development.js:23964)

Digging in, it looks like the remark-code-extra plugin is returning Promises, forcing the async flow.

Runtime

No response

Package manager

No response

OS

No response

Build and bundle tools

No response

wooorm commented 2 years ago

PR welcome!

github-actions[bot] commented 2 years ago

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

wooorm commented 2 years ago

Also, probably a very bad idea to use a plugin that hasn’t been updated in 3 years...

mikesir87 commented 2 years ago

Also, probably a very bad idea to use a plugin that hasn’t been updated in 3 years...

Haha... very true. I saw that the plugins were referenced on the list of plugins and still indicated they were "up to date". I guess that means different things to different people! 😆

I might take a stab at a PR, but maybe it's worth looking around to see if there are any newer plugins too. Thanks!

wooorm commented 2 years ago

That indication means that it did not start crashing when remark introduced a breaking change two 2 years ago, which did affect some plugins.

maybe it's worth looking around to see if there are any newer plugins too

Perhaps I can help you if I know what you are looking for?

mikesir87 commented 2 years ago

Perhaps I can help you if I know what you are looking for?

I like the idea of using the remark-code-frontmatter plugin and loading additional metadata into the code snippets. My use case is displaying code snippets, but having additional metadata to enable various features (when to show a copy button, a sample filename to display, etc.). Combining that with the remark-code-extra has made it super simple to wrap the code blocks.

FWIW... it's been working great and I haven't run into issues yet beyond needing the processor to run asynchronously. But, if there are other plugins you think can meet this need, I'll be happy to switch!

ujjwalchadha8 commented 1 year ago

This is definitely a blocker for many scenarios and going to be a bigger one once react server components are more common and people would wanna put async code in plugins. I've added more context on the PR

wooorm commented 1 year ago

blocker for many scenarios

Such as?

once react server components are more common and people would wanna put async code in plugins

Why? I don’t see how RSC affects this issue.

ujjwalchadha8 commented 1 year ago

Why? I don’t see how RSC affects this issue.

I am trying to build a remark plugin which requires async run and use it in react markdown. I am using this in a React Server Component

If this was a client-side component, I would have called the plugin in a hook (as a standalone remark plugin) and loaded the result in the react markdown plugin when the result was complete, allowing me to run the code synchronously

Since this is a server-side component, the result needs to be compiled on the server and I cannot use hooks or state. It would make things easier if the component (ReactMarkdown) itself is made asynchronous, allowing it to use run instead of runSync for plugins.

Such as?

With RSC, more people are going to use ReactMarkdown to render markdown on server side instead of using other technologies because it integrates with their frontend well and it is easier to maintain it in a single codebase. In such cases, async plugins can potentially be more common. This is the exact same thing I am facing. I was previously rendering md on server side manually with some limited format options. Now I can just use ReactMarkdown with RSC

wooorm commented 1 year ago

I am using this in a React Server Component

Can you show this component?

itself is made asynchronous

Can you show docs on these async components that you are talking about?

ujjwalchadha8 commented 1 year ago

Here is the link to the RFC for React Server Components: https://github.com/reactjs/rfcs/blob/main/text/0188-server-components.md

Read the example: https://github.com/reactjs/rfcs/blob/main/text/0188-server-components.md#basic-example Notice the difference between the first example (server component which is async) and the second example (client component)

Can you show this component?

The remark plugin I am building basically reads a link node in the markdown (mdast tree), then performs http requests on that URL to fetch some data. Based on that data, it changes the mdast subtree of that link node. So my plugin must run async.

wooorm commented 1 year ago

As I understand it, the RFC on async/await on the server is still open (https://github.com/reactjs/rfcs/pull/229). I believe this is also what adds a use() to the client to await promises. And there are a lot of comments there about naming.

faisalsayed10 commented 1 year ago

Any updates on this issue yet? Trying to use shiki with remark but getting the same async errors

wooorm commented 1 year ago

Folks, you’re spamming. Come and help solve this issue (and the upstream React issue) or hush.

richardr1126 commented 11 months ago

Folks, you’re spamming. Come and help solve this issue (and the upstream React issue) or hush.

Then maybe there shouldn't be a link to a list of plugins that are said to work, but don't, in the README file.

skyl commented 5 months ago

It appears this is what I'm experiencing with remark-mermaidjs. @4 it doesn't require async, so that works, with react-markdown@8. But, react-markdown@9 has a different interface that is not compatible with remark-mermaidjs@4

react-markdown	remark-mermaidjs	compliles	works?
8	4	✅	✅
9	4	❌	❌
9	6	✅	❌

with the latest react-markdown and remark-mermaidjs results in:

`runSync` finished async. Use `run` instead

This should be easy to reproduce:

import ReactMarkdown from "react-markdown"
import remarkMermaidjs from 'remark-mermaidjs';

<ReactMarkdown
            className="math-markdown"
            children={props.content}
            remarkPlugins={[
                remarkMermaidjs,
            ]}
            // ...
/>

wooorm commented 5 months ago

@skyl first see the big note in the remark-mermaidjs readme: don’t use it, use something else. Second, yes, the plugin in its changelog says that it changed from sync to async. I’ll hide your post because it doesn’t add something on how to implement the requested feature

remcohaszing commented 5 months ago

Here’s an idea:

We add a new prop async. If this is false (default), use the current behaviour. However, if this is true, we use an async renderer.

react-markdown provides 2 async renderers.

One async renderer uses async/await. This is compatible with server side rendered components.
The other uses useEffect and useState to handle any asynchronous logic. This is a bit more complex, but not too hard. If there is an error, this error is set in the state and thrown synchronously on render. This is not the most elegant way to handle errors, but it’s compatible with the other component.

Which async renderer to use, is determined using export conditions.

wooorm commented 5 months ago

I think that’s the idea floated around in https://github.com/remarkjs/react-markdown/pull/682#issuecomment-1766899204 too?

remcohaszing commented 5 months ago

I see. I remember we talked about it, but I didn’t see it in this issue and didn’t realize the idea was posted in that PR.

Is a PR welcome for that, or is more discussion needed?

wooorm commented 5 months ago

Yes, looking for someone to do the work and investigate whether it works or not. Not sure about the prop you mention here. And I believe it to be hard to test and dependent on undocumented/experimental features (https://github.com/reactjs/rfcs/pull/229) so that’s why I think marking the API as experimental and unstable for now is a good idea.

remarkjs / react-markdown