Support async components #2242
Comments
|
|
Right, those are all good points. It makes sense to make this opt-in, I imagine an option named This will also break editor support, not because of React or MDX, but because of the same reason async components aren’t supported in TypeScript (microsoft/TypeScript#21699). I personally don’t need this right now, it’s just an idea I had. We can wait until someone has a real use case. |
|
@remcohaszing I think there are too many ifs currently, this isn’t actionable until React (and others!) solve their promise story. We can discuss again in the future. Can I close this? |
|
Maybe I mixed up some terminology and supporting async components isn't the same as supporting RSC. But from the perspective of MDX this is about supporting async components. Next.js supports already async components in Also the related TypeScript issue has been resolved. I'd say we can wait for user feedback to see if users actually want to use this. Based on the fact there are already some 👍 reactions, I think they do. |
|
We don’t just support Next, or React, but any arbitrary runtime. |
I think this proves my point. We support any arbritrary runtime. This runtime may support async function components. Why should MDX not support that? Also to clarify, my proposal is to only make the component async if the body contains |
|
But nobody supports it yet? It doesn’t seem like a great idea to design things that can’t be tested.
How are they not related? We already parse |
Next.js does. For example see https://nextjs.org/docs/app/building-your-application/data-fetching/fetching-caching-and-revalidating#fetching-data-on-the-server-with-fetch It would make
- export type MDXContent = (props: MDXProps) => JSX.Element;
+ export type MDXContent = FunctionComponent<MDXProps>; |
I don’t classify that as the “yet” part. They support RSC. The RFC isn’t merged, see first comment here: #2242 (comment).
And what if a layout is used?
And what if someone is using Preact or Vue or anything other than Next? The types wouldn’t match reality. They could get |
Then
The return type of MDX’s |
|
Checking this out locally. I think we’d only need to detect for Would we also have to use async fn for that? 🤔 |
+++ b/packages/mdx/lib/plugin/recma-document.js
@@ -602,9 +602,23 @@ export function recmaDocument(options) {
argument = argument.children[0]
}
+ let awaitExpression = false
+
+ walk(argument, {
+ enter(node) {
+ if (
+ node.type === 'AwaitExpression' ||
+ (node.type === 'ForOfStatement' && node.await)
+ ) {
+ awaitExpression = true
+ }
+ }
+ })
+
return [
{
type: 'FunctionDeclaration',
+ async: awaitExpression,
id: {type: 'Identifier', name: '_createMdxContent'},
params: [{type: 'Identifier', name: 'props'}],
body: {
import fs from 'node:fs/promises'
import {compile} from '@mdx-js/mdx'
import {createElement} from 'react'
import {renderToString} from 'react-dom/server'
const document = `{await Promise.resolve(42)}`
const result = String(await compile(document))
await fs.writeFile('output.js', result)
console.log('code:', result)
const id = './output.js#' + Date.now()
/** @type {import('mdx/types.js').MDXModule} */
const mod = await import(id)
const Content = mod.default
console.log('Content:', Content)
const output = renderToString(createElement(Content))
console.log('output:', output)Yields: Interesting. All tests pass too. But eh. Useless? And no type errors? |
|
Yes, we’d need to check for function _createMdxContent() {
await 1 // ← top-level
if (await 1) { // if-statement
await 1
}
while (await 1) { // loop
await 1
}
const foo = {
bar: await 1 // nested property
}
foo(await 1) // as argument
async function foo() {
await 1 // Inside an embedded function doesn’t count.
}
}This is doable, but it does look a bit more involved than I initially anticipated. |
|
Why? I posted working code above! |
While I was fiddling. I think your code works, but needs to stop traversion on a function expression, function declaration, or arrow function. |
|
Ah, it should also check async functions! |
|
wait no, it should indeed skip, and not use |
|
I thought maybe async components only work with streams, since you can’t go from an async function to a synchronous import fs from 'node:fs/promises'
import {compile} from '@mdx-js/mdx'
import {createElement} from 'react'
import server from 'react-dom/server'
const document = `{await Promise.resolve(42)}`
const result = String(await compile(document))
await fs.writeFile('output.js', result)
console.log('code:', result)
const id = './output.js#' + Date.now()
/** @type {import('mdx/types.js').MDXModule} */
const mod = await import(id)
const Content = mod.default
console.log('Content:', Content)
console.dir(server)
server.renderToPipeableStream(createElement(Content)).pipe(process.stdout) |
|
Note that the change turns what would now emit a syntax error to support for a new feature without breaking changes. This could be done later in a semver minor. |
wooorm
added
💪 phase/solved
|
Not sure about the types in |
|
We could either leave it as-is, or make the changes I proposed in #2242 (comment). For now I think it's best to keep the types as-is for compatibility with TypeScript 5.0 and lower. |
|
fine! |
Initial checklist
Problem
React server components can be asynchronous. It would be nice if MDX content supports this too.
Solution
If an
awaitexpression is detected, mark_createMdxContentas async.Alternatives
We could not support it 🤷
The text was updated successfully, but these errors were encountered: