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

Some plugins require async run #680

Open
4 tasks done
mikesir87 opened this issue Apr 5, 2022 · 24 comments · May be fixed by #682
Open
4 tasks done

Some plugins require async run #680

mikesir87 opened this issue Apr 5, 2022 · 24 comments · May be fixed by #682
Labels
👍 phase/yes Post is accepted and can be worked on

Comments

@mikesir87
Copy link

Initial checklist

Affected packages and versions

8.0.2

Link to runnable example

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

Steps to reproduce

  1. Install remark-code-frontmatter
  2. Install remark-code-extra
  3. Configure the component to use both plugins
  4. 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

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Apr 5, 2022
@wooorm
Copy link
Member

wooorm commented Apr 5, 2022

PR welcome!

@wooorm wooorm added the 👍 phase/yes Post is accepted and can be worked on label Apr 5, 2022
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Apr 5, 2022
@github-actions
Copy link

github-actions bot commented Apr 5, 2022

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
Copy link
Member

wooorm commented Apr 5, 2022

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

@mikesir87
Copy link
Author

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!

mikesir87 added a commit to mikesir87/react-markdown that referenced this issue Apr 6, 2022
Resolves remarkjs#680

Signed-off-by: Michael Irwin <mikesir87@gmail.com>
@mikesir87 mikesir87 linked a pull request Apr 6, 2022 that will close this issue
5 tasks
@wooorm
Copy link
Member

wooorm commented Apr 6, 2022

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
Copy link
Author

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
Copy link

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
Copy link
Member

wooorm commented Dec 2, 2022

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
Copy link

ujjwalchadha8 commented Dec 3, 2022

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
Copy link
Member

wooorm commented Dec 4, 2022

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
Copy link

ujjwalchadha8 commented Dec 5, 2022

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
Copy link
Member

wooorm commented Dec 6, 2022

As I understand it, the RFC on async/await on the server is still open (reactjs/rfcs#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

This comment was marked as spam.

@wooorm
Copy link
Member

wooorm commented Jun 13, 2023

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

@richardr1126

This comment was marked as spam.

@skyl

This comment was marked as off-topic.

@wooorm
Copy link
Member

wooorm commented Jan 29, 2024

@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
Copy link
Member

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.

  1. One async renderer uses async/await. This is compatible with server side rendered components.
  2. 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
Copy link
Member

wooorm commented Feb 13, 2024

I think that’s the idea floated around in #682 (comment) too?

@remcohaszing
Copy link
Member

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
Copy link
Member

wooorm commented Feb 13, 2024

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 (reactjs/rfcs#229) so that’s why I think marking the API as experimental and unstable for now is a good idea.

@programming-with-ia

This comment was marked as spam.

@programming-with-ia

This comment was marked as spam.

@liuhq
Copy link

liuhq commented Nov 7, 2024

also the async problem from using shiki that is a code highlighter -> https://shiki.matsu.io/packages/rehype

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👍 phase/yes Post is accepted and can be worked on
Development

Successfully merging a pull request may close this issue.

9 participants