Fix empty 'On this page' menu caused by duplicate/empty heading IDs (#352)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: simonhamp

app/Support/CommonMark/CommonMark.php

@@ -20,12 +20,18 @@ class CommonMark
 {
     protected static ?MarkdownConverter $converter = null;
 
+    protected static ?HeadingRenderer $headingRenderer = null;
+
     public static function convertToHtml(string $markdown, array $data = []): string
     {
         // Pre-process to render any Blade components in the markdown
         $markdown = BladeMarkdownPreprocessor::process($markdown, $data);
 
-        return static::getConverter()->convert($markdown)->getContent();
+        // Reset heading ID tracking to ensure unique IDs per conversion
+        static::getConverter();
+        static::$headingRenderer->resetIds();
+
+        return static::$converter->convert($markdown)->getContent();
     }
 
     protected static function getConverter(): MarkdownConverter
@@ -45,7 +51,8 @@ protected static function getConverter(): MarkdownConverter
 
             $environment->addExtension(new CommonMarkCoreExtension);
             $environment->addExtension(new GithubFlavoredMarkdownExtension);
-            $environment->addRenderer(Heading::class, new HeadingRenderer);
+            static::$headingRenderer = new HeadingRenderer;
+            $environment->addRenderer(Heading::class, static::$headingRenderer);
             $environment->addExtension(new TableExtension);
 
             $environment->addExtension(new EmbedExtension);

app/Support/CommonMark/HeadingRenderer.php

@@ -10,6 +10,14 @@
 
 class HeadingRenderer implements NodeRendererInterface
 {
+    /** @var array<string, int> */
+    protected array $usedIds = [];
+
+    public function resetIds(): void
+    {
+        $this->usedIds = [];
+    }
+
     public function render(Node $node, ChildNodeRendererInterface $childRenderer)
     {
         $tag = 'h'.$node->getLevel();
@@ -20,6 +28,17 @@ public function render(Node $node, ChildNodeRendererInterface $childRenderer)
 
         $id = Str::slug($element->getContents());
 
+        if ($id === '') {
+            $id = 'heading';
+        }
+
+        if (isset($this->usedIds[$id])) {
+            $this->usedIds[$id]++;
+            $id = $id.'-'.$this->usedIds[$id];
+        } else {
+            $this->usedIds[$id] = 0;
+        }
+
         $element->setAttribute('id', $id);
 
         if ($node->getLevel() === 1 || $node->getLevel() === 2 || $node->getLevel() === 3) {

resources/views/components/plugin-toc.blade.php

@@ -5,16 +5,28 @@
             const article = document.querySelector('article')
             if (! article) return
 
+            const seen = new Set()
             const elements = article.querySelectorAll('h2[id], h3[id]')
-            this.headings = Array.from(elements).map(el => {
-                const clone = el.cloneNode(true)
-                clone.querySelectorAll('.heading-anchor').forEach(a => a.remove())
-                return {
-                    id: el.id,
-                    text: clone.textContent.trim(),
-                    level: parseInt(el.tagName.substring(1)),
-                }
-            })
+            this.headings = Array.from(elements)
+                .filter(el => el.id !== '')
+                .map(el => {
+                    const clone = el.cloneNode(true)
+                    clone.querySelectorAll('.heading-anchor').forEach(a => a.remove())
+
+                    let id = el.id
+                    let suffix = 1
+                    while (seen.has(id)) {
+                        id = el.id + '-toc-' + suffix++
+                    }
+                    seen.add(id)
+
+                    return {
+                        id: el.id,
+                        tocKey: id,
+                        text: clone.textContent.trim(),
+                        level: parseInt(el.tagName.substring(1)),
+                    }
+                })
         },
     }"
     x-show="headings.length > 0"
@@ -28,7 +40,7 @@
 
         <flux:popover class="w-64">
             <nav class="flex max-h-80 flex-col gap-0.5 overflow-y-auto">
-                <template x-for="heading in headings" :key="heading.id">
+                <template x-for="heading in headings" :key="heading.tocKey">
                     <a
                         :href="'#' + heading.id"
                         x-on:click.prevent="document.getElementById(heading.id)?.scrollIntoView({ behavior: 'smooth', block: 'start' })"

tests/Feature/HeadingRendererTest.php

@@ -57,4 +57,34 @@ public function test_h4_does_not_get_anchor(): void
 
         $this->assertStringNotContainsString('heading-anchor', $html);
     }
+
+    public function test_duplicate_headings_get_unique_ids(): void
+    {
+        $html = CommonMark::convertToHtml("## Installation\n\nSome text.\n\n## Installation");
+
+        preg_match_all('/id="([^"]+)"/', $html, $matches);
+        $ids = $matches[1];
+
+        $this->assertCount(2, $ids);
+        $this->assertCount(2, array_unique($ids), 'Heading IDs should be unique');
+        $this->assertSame('installation', $ids[0]);
+        $this->assertSame('installation-1', $ids[1]);
+    }
+
+    public function test_empty_slug_gets_fallback_id(): void
+    {
+        // A heading with only special characters that Str::slug strips
+        $html = CommonMark::convertToHtml('## !!!');
+
+        $this->assertStringContainsString('id="heading"', $html);
+    }
+
+    public function test_ids_reset_between_conversions(): void
+    {
+        CommonMark::convertToHtml('## Installation');
+        $html = CommonMark::convertToHtml('## Installation');
+
+        $this->assertStringContainsString('id="installation"', $html);
+        $this->assertStringNotContainsString('id="installation-1"', $html);
+    }
 }