-
-
Notifications
You must be signed in to change notification settings - Fork 874
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
Add async processing support to support async plugins #682
base: main
Are you sure you want to change the base?
Conversation
Resolves remarkjs#680 Signed-off-by: Michael Irwin <mikesir87@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Thanks for the PR! Hmm, 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.
My bad. I'll get on that. 😄 I did at least update the typings for the new attribute.
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?
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
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
Based on my limited experience, it does appear that server-side rendering is unsupported for My guessing is that, based on me having to use the |
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
ebf016d
to
bc044a7
Compare
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
Docs updated and added a simple async plugin to validate it both executes and the overall rendering still works. |
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. |
There are still some more todos from #682 (comment) as I understand it. |
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 |
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 This also means that the server-side components cannot have 'useState', which means the PR probably needs to change to support server side components. |
Why?
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? |
This comment was marked as spam.
This comment was marked as spam.
@reypm you can find the status by reading the comments directly above yours. |
Hey @wooorm @ChristianMurphy! How can I help land this PR? |
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. |
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 |
Got it! I ended up just using |
Is there any progress or blockers on this one? I've encountered it a few times now: initially with a plugin I'm developing, and now again with rehypejs/rehype-starry-night |
see the earlier comments please :) |
Investigated this for a bit last week, an |
How about the |
Resolves #680
Initial checklist
Description of changes
Added an
async
option to the component. If true, runs the renderer usingprocessor.run
instead ofprocessor.runSync
(which causes an error be thrown).