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.

Sign up for GitHub

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

Jump to bottom

Server Islands not rendering as islands in .mdx files #12252

Open
1 task
ColeTownsend opened this issue Oct 17, 2024 · 5 comments · May be fixed by #12574
Open
1 task

Server Islands not rendering as islands in .mdx files #12252

ColeTownsend opened this issue Oct 17, 2024 · 5 comments · May be fixed by #12574
Assignees
@matthewp
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: server islands Related to Server Islands (scope)

Comments

@ColeTownsend
Copy link

ColeTownsend commented Oct 17, 2024 •
edited
Loading

Astro Info

Astro                    v4.16.5
Node                     v22.9.0
System                   macOS (arm64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/cloudflare
Integrations             @astrojs/mdx
                         @astrojs/react
                         astro-meta-tags
                         @astrojs/tailwind
                         pagefind

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Astro components imported into .mdx files that use prop server:dever are rendered the same way as the page, i.e. dynamically or prerendered. I'm experiencing this using cloudflare adapter, but for the example I had to use nodejs for stackblitz.

Server islands should not delay the entire page render in dev mode (seems to cause waterfall), and when pre-rendered should not be output statically.

Example

You can observer within the dist folder under client the entire page is rendered to static html, including the server island.

image

What's the expected result?

The server island should behave the same as it does within an .astro file.

Link to Minimal Reproducible Example

Note: you will need to run npm start to generate a preview build.
https://stackblitz.com/edit/github-njjrmi?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Oct 17, 2024
@matthewp matthewp added - P4: important Violate documented behavior or significantly impacts performance (priority) feat: server islands Related to Server Islands (scope) labels Oct 17, 2024
@matthewp matthewp self-assigned this Oct 17, 2024
@matthewp matthewp removed the needs triage Issue needs to be triaged label Oct 17, 2024
@tombennet
Copy link

Will this be fixed in the 5.0.0 release? i.e. are server islands intended to function the same in content (.mdx) and .astro files?

@matthewp
Copy link
Contributor

matthewp commented Nov 6, 2024

The appears to work in dev mode, so the bug only appears in prod, is that right?

@matthewp
Copy link
Contributor

matthewp commented Nov 6, 2024

This appears to be working for me. This is in prod, after a build. What am I missing?

serverdefer.mp4

@ColeTownsend
Copy link
Author

ColeTownsend commented Nov 7, 2024 •
edited
Loading

@matthewp click through to the blog post (the mdx file), and you will see that the Server Island within it is simply rendered statically. The home page server island is just to show that normal files work.

When a normal astro file (index.astro) is prerendered, the server island works as expected. In /blog/[slug].astro, the server island is simply rendered statically.

image

@apatel369
Copy link
Contributor

I was able to reproduce this issue in both the dev and prod builds. Upon further debugging, I found that the server islands metadata is not being added for JSX components in MDX files. The relevant code is in rehype.ts.

@matthewp, should I create a PR to add the server islands metadata?

@apatel369 apatel369 mentioned this issue Nov 15, 2024
Closed
apatel369 added a commit to apatel369/astro that referenced this issue Nov 15, 2024
@apatel369
Merge branch 'next' into arpan-withastro#12252-mdx-islands
e63f5a2
matthewp added a commit to apatel369/astro that referenced this issue Nov 26, 2024
@matthewp
Merge branch 'next' into arpan-withastro#12252-mdx-islands
ff01d35
@apatel369 apatel369 linked a pull request Nov 30, 2024 that will close this issue
Draft
apatel369 added a commit to apatel369/astro that referenced this issue Nov 30, 2024
@apatel369
Merge branch 'main' into arpan-withastro#12252-mdx-islands
14ba50f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthewp matthewp

Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: server islands Related to Server Islands (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add server islands support for MDX apatel369/astro
4 participants
@matthewp @ColeTownsend @tombennet @apatel369