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

Bug with MDX optimization - set:html="" #11971

Closed
1 task
stereobooster opened this issue Sep 11, 2024 · 2 comments · Fixed by #11975
Closed
1 task

Bug with MDX optimization - set:html="" #11971

stereobooster opened this issue Sep 11, 2024 · 2 comments · Fixed by #11975
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) pkg: mdx Issues pertaining to `@astrojs/mdx` integration

Comments

@stereobooster
Copy link

stereobooster commented Sep 11, 2024

Astro Info

Astro                    v4.15.4
Node                     v18.17.1
System                   macOS (x64)
Package Manager          yarn
Output                   static
Adapter                  none
Integrations             @astrojs/starlight
                         @astrojs/mdx

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

n/a

Describe the Bug

When mdx optimization turned on (default in starlight 0.23+) it seems it doesn't correctly render tag in MDX

<figure class="beoe mermaid not-content" set:html="<div></div>"></figure>

I know that documentation says that it may break behavior of custom tags. But I think in this specific case this is a bug - all html is there, it just "forgot" to move content from set:html inside the tag

Related stereobooster/beoe#2

What's the expected result?

So it would actually output contents from set:html inside the tag

Link to Minimal Reproducible Example

https://github.com/jarodtaylor/starlight-mermaid

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 Sep 11, 2024
@bluwy
Copy link
Member

bluwy commented Sep 12, 2024

I've sent a fix for this, however there's a bug in @beoe/rehype-mermaid that is injecting root hast nodes inside a tree, which is incorrect following the spec:

Root can be used as the root of a tree, or as a value of the content field on a 'template' Element, never as a child.

And it trips up the MDX optimization as it didn't expect this case. However, since MDX still handles fine with it, I fixed it anyway to not cause any inconsistencies.

Still, it would be best if you update your plugin to not inject the root node. You can take its children and inject that instead. I tested with the main rehype-mermaid package and it didn't have this issue.

@bluwy bluwy added - P3: minor bug An edge case that only affects very specific usage (priority) pkg: mdx Issues pertaining to `@astrojs/mdx` integration and removed needs triage Issue needs to be triaged labels Sep 12, 2024
@stereobooster
Copy link
Author

Root can be used as the root of a tree, or as a value of the content field on a 'template' #11971 (comment), never as a child.

Makes sense. Thanks for pointing out this issue - I will fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) pkg: mdx Issues pertaining to `@astrojs/mdx` integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants