withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.96k stars 2.5k forks source link

[MDX] `headings` prop not being properly populated with nested `<Content />` #10447

Closed Hugos68 closed 8 months ago

Hugos68 commented 8 months ago

Astro Info

Astro                    v4.5.4
Node                     v20.11.0
System                   Windows (x64)
Package Manager          pnpm
Output                   static
Adapter                  none
Integrations             @astrojs/mdx

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

No response

Describe the Bug

It's easier to explain the issue with a quick example, let's say we have the following:

Layout.astro

<slot />

foo.mdx

## Foo

bar.mdx

---
layout: ./Layout.astro
---

import { Content } from './foo.mdx';

<Content />

## Bar

When rendering the bar.mdx file we get the following: image

This is great, we now have a reusable markdown file: foo.mdx that we can import and use in multiple places.

Now we want to generate a Table Of Contents for bar.mdx.

Astro's MDX integration has a built in feature that passes a headings prop into the layout that the MDX files are using, this means that Layout.astro will have a headings prop available.

This is what we get back when doing console.log(Astro.props.headings):

[ 
  { 
    depth: 2, 
    slug: 'bar', 
    text: 'Bar' 
  } 
]

This is weird, we are only getting back 1 heading while the page actually has 2 headings.

This happens because the headings prop from Astro is populated through a rehype/remark plugin that only checks for headings in the .mdx/.md file that is being processed, but it does not check for any imported .mdx/md content that could also have headings, for example:

import { Content } from 'foo.mdx';

// This heading is picked up because it is a `node` of type `heading`.
# I am a heading 

// This heading inside `Content` is *not* picked up because it isn't a `node` of type `heading` even though it could still contain markdown headings.
<Content /> 

Docusaurus, a documentation website builder similar to Starlight, had the same issue which can be viewed here: https://github.com/facebook/docusaurus/issues/3915

They ended up solving it by recursively importing and scanning markdown inside their plugin to grab any nested headings, the same solution could be applied here.

I've actually created a proof of concept plugin here which is in a working state but fails to cover some edge cases: https://github.com/Hugos68/remark-astro-headings

What's the expected result?

When importing .mdx/.md I expect the headings prop to be populated correctly accounting for headings that might be inside imported <Content />.

Link to Minimal Reproducible Example

https://github.com/Hugos68/astro-headings-issue-repro

Participation

matthewp commented 8 months ago

That solution doesn't work with how Astro works. Astro components compile to a function similar to this:

function App() {
  const props = Astro.props;

  return html`<h1>Title</h1>
    <OtherComponent />
  `
}

I hope that illustrates things. We don't render the component and then hand you props, the props are used to determine what to render. So I don't think we could implement what you're hoping for here.

Hugos68 commented 8 months ago

Hey @matthewp, first of all, thanks for the quick response! If this is something that cannot be done due to technical limitations on Astro's side then that's unfortunate, I really don't know enough about Astro's internals either to think of any ways to solve this. I did however learn than in the meanwhile I can do:

---
import * as Foo from "./foo.mdx";
import * as Bar from "./bar.mdx";

const merged = Foo.getHeadings().concat(Bar.getHeadings());

console.log(merged);
---

<Foo.Content />
<Bar.Content />

Which results in:

[
  { depth: 2, slug: 'foo', text: 'Foo' },
  { depth: 2, slug: 'bar', text: 'Bar' }
]

It isn't the best solution because you need to manually retrieve and merge headings but it's atleast something that I can use for now. I see you've labeled it as requiring discussion so I'll just wait for that.

matthewp commented 8 months ago

We decided this is a documentation issue and will be updating the docs on MDX.

matthewp commented 8 months ago

PR is here: https://github.com/withastro/docs/pull/7409

Closing as this is not an Astro bug. Glad you found another way!