feat: add server islands support for MDX
#12574
Conversation
🦋 Changeset detectedLatest commit: 41e55b2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
apatel369
changed the title
apatel369
changed the title
MDX
CodSpeed Performance ReportMerging #12574 will not alter performanceComparing Summary
|
|
@apatel369 any updates? |
|
@ematipico I applied the changes suggested by @matthewp. Tests are still failing. |
|
@apatel369 the PR was in draft for a long time without any updates. What suggestions are you referring to? What kind of help do you need? |
github-actions
bot
removed
the
pkg: example
| } | ||
| }, | ||
| transform(_code, id) { | ||
| if (id.endsWith('.astro')) { |
There was a problem hiding this comment.
This was preventing retrieving server islands data from mdx files metadata
| props[key] = value; | ||
| } | ||
| } | ||
| const str = await renderToString(result, vnode.type as any, props, slots); |
There was a problem hiding this comment.
This was calling the component factory as is. Instead, we want it to go through renderAstroComponent so the server island special handling can take effect
| ...meta, | ||
| astro: { | ||
| ...(meta.astro ?? { hydratedComponents: [], clientOnlyComponents: [], scripts: [] }), | ||
| ...(meta.astro ?? createDefaultAstroMetadata()), |
There was a problem hiding this comment.
Now that the server island plugin is running for all files, this was failing because the metadata was not initialized with serverComponents: []
There was a problem hiding this comment.
Just confirming that no docs update is needed:
- there's nothing in docs that explicitly says this wasn't possible before, or that server islands could only be rendered in
.astrocomponents - all current wording re: MDX refers to the ability to "use Astro components", which server islands are.
| logger.error( | ||
| 'islands', | ||
| 'You tried to render a server island without an adapter added to your project. An adapter is required to use the `server:defer` attribute on any component. Your project will fail to build unless you add an adapter or remove the attribute.', | ||
| ); |
There was a problem hiding this comment.
| logger.error( | |
| 'islands', | |
| 'You tried to render a server island without an adapter added to your project. An adapter is required to use the `server:defer` attribute on any component. Your project will fail to build unless you add an adapter or remove the attribute.', | |
| ); | |
| logger.error( | |
| 'islands', | |
| 'You tried to render a server island without an adapter added to your project. An adapter is required to use the `server:defer` attribute on any component. Your project will fail to build unless you add an adapter or remove the attribute.', | |
| ); | |
| continue; |
We should not process the island, I think. Make sure to print the name of the component, so the users know which component island is failing.
There was a problem hiding this comment.
I don't think we should stop the processing. We have this in a few places IIRC where we only warn if stuff is going to break if no adapter is added, and only break during build. I think that applies here too
Co-authored-by: Matthew Phillips <[email protected]> Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: Florian Lefebvre <[email protected]>
5 participants
Changes
.mdxfiles #12252Testing
Manual and new fixture test
Docs
Changeset