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

MDX variable headings display badly in TOC #8240

Closed
1 task
VojtaStruhar opened this issue Aug 24, 2023 · 6 comments
Closed
1 task

MDX variable headings display badly in TOC #8240

VojtaStruhar opened this issue Aug 24, 2023 · 6 comments
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) needs discussion Issue needs to be discussed pkg: mdx Issues pertaining to `@astrojs/mdx` integration

Comments

@VojtaStruhar
Copy link

VojtaStruhar commented Aug 24, 2023

What version of starlight are you using?

0.7.3

What version of astro are you using?

2.10.5

What package manager are you using?

npm

What operating system are you using?

Mac

What browser are you using?

Safari

Describe the Bug

If I put a javascript variable as a heading in MDX, it looks fine on the page, but bad in the sidebar:

export const variableName = "Javascript heading";

## {variableName}

Results in Javascript heading written on the page, but variableName in the sidebar.

Screenshot

With a function that returns a string, it writes something like functionName in the sidebar - but again works well on the main page.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-i3eo86?file=src%2Fcontent%2Fdocs%2Findex.mdx

Participation

  • I am willing to submit a pull request for this issue.
@delucis
Copy link
Member

delucis commented Aug 26, 2023

Thanks for the issue @VojtaStruhar! We rely on Astro's conversion of your content to a list of headings so will have to check with the team whether this is something they can fix upstream.

Will move this issue to the Astro repo for now as that is where we'd need to fix it.


Some more context for whoever triages this: Starlight uses the headings property returned by entry.render() to generate its table of contents. Headings are collected by Astro with a remark plug-in, but MDX expressions haven't been evaluated at that point, so display as their code version instead of the expression result.

To fix this, we'd have to use the heading contents after MDX has finished rendering.

One complication is that some people rely on literal HTML headings not being included in Astro's headings array (e.g. # heading is included, <h1>heading</h1> is not). We do in docs for example and it's also the behaviour used by Docusaurus to make it easy to opt out of the table of contents by switching to HTML syntax. This means we can't simply move the heading collection step later, because that would be when we can't tell the difference between HTML and Markdown syntax. So this will need some thought to be able to support.

@delucis delucis transferred this issue from withastro/starlight Aug 26, 2023
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Aug 26, 2023
@natemoo-re natemoo-re added pkg: mdx Issues pertaining to `@astrojs/mdx` integration - P3: minor bug An edge case that only affects very specific usage (priority) labels Aug 31, 2023
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Aug 31, 2023
@natemoo-re
Copy link
Member

I'm not sure where we ever ended up on this. I think at one time @bholmesdev had fixed this logic, but I might not be remembering correctly. It's possible that this is just a limitation of using MDX.

We'll check and keep this issue updated!

@natemoo-re natemoo-re added the needs discussion Issue needs to be discussed label Aug 31, 2023
@delucis
Copy link
Member

delucis commented Sep 5, 2023

Thanks Nate — I think we may have had this working in Astro-Flavored Markdown back in the day, but never in MDX? It’s not impossible to support, but likely convoluted and any solution I can imagine does risk conflicting with user-defined plugins.

Question for @VojtaStruhar: what’s your use case for variable headings? Curious what need you have that makes them necessary in case that helps us understand a better fix.

@VojtaStruhar
Copy link
Author

@delucis Thanks for the question. I wanted to use MDX for generating content. Let's say reading the file system to list all plugins present in a project. It works perfectly fine, but the headings haven't been put into TOC. This led me to try to mess around with generating headings and consequently I tried this JS heading thing.

The idea was that I could have

  • documentation (by hand)
  • generated heading (JS function)
  • generated content (JSX)
  • documentation (by hand)

In the TOC I thought I'd settle for a single heading to mark the section (because the JSX stuff doesn't appear at all). And again - on the page itself it works and looks fine! But in the TOC just the name of the function shows.

By now I'm very confused when exactly does the TOC processing happen 😄

@bholmesdev
Copy link
Contributor

bholmesdev commented Nov 16, 2023

Thanks @VojtaStruhar! Sorry for the radio silence here. I looked at the source code and it seems we evaluate JSX expressions when they reference frontmatter properties, but not when they reference MDX variable exports:

---
title: My Title With Appear in TOC
---

export const variable = "But this won't!"

# {title}

## {variable}

This is because frontmatter is processed before Markdown is rendered, but MDX exports are resolved at the final rendering step. To answer your question on what runs when:

  1. YAML frontmatter is resolved
  2. Remark plugins are run over Markdown tree
  3. Rehype plugins are run over HTML tree (this is where the TOC is generated!)
  4. MDX JS expressions are rendered

We collect headings as part of the rehype process so users can apply their own rehype plugins to customize headings before or after the TOC is generated. If we wait for the MDX postprocessing step, this feature is lost.

I won't say this is impossible though! Now that I'm seeing our JSX expression resolver, I think a userland plugin could achieve this at least. I'm unfortunately pulled away from Astro core through the end of the year but I'll forward this along to the team. You're also welcome to explore the source code here if you want to try a fix 😄

@bholmesdev
Copy link
Contributor

Closing this as a "will not fix" due to inactivity and potential userland fix

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) needs discussion Issue needs to be discussed pkg: mdx Issues pertaining to `@astrojs/mdx` integration
Projects
None yet
Development

No branches or pull requests

4 participants