Skip to content

fix: support mutations from child nodes for dynamic TOC#2363

Merged
SoonIter merged 1 commit intomainfrom
fix/dynamic-toc
Jul 10, 2025
Merged

fix: support mutations from child nodes for dynamic TOC#2363
SoonIter merged 1 commit intomainfrom
fix/dynamic-toc

Conversation

@JounQin
Copy link
Copy Markdown
Collaborator

@JounQin JounQin commented Jul 10, 2025

Summary

support mutations from child nodes for dynamic TOC

also refactor to use for of to reduce loop execution

Related Issue

close #2346

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@JounQin JounQin requested review from SoonIter and Copilot July 10, 2025 07:16
@JounQin JounQin self-assigned this Jul 10, 2025
@JounQin JounQin added the 🐞 bug Something isn't working label Jul 10, 2025
@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 10, 2025

Deploy Preview for rspress ready!

Name Link
🔨 Latest commit 65342b1
🔍 Latest deploy log https://app.netlify.com/projects/rspress/deploys/686f6b840738540008c8f506
😎 Deploy Preview https://deploy-preview-2363--rspress.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 78 (🔴 down 18 from production)
Accessibility: 97 (no change from production)
Best Practices: 92 (🟢 up 9 from production)
SEO: 100 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 10, 2025

Deploy Preview for rspress-v2 ready!

Name Link
🔨 Latest commit 65342b1
🔍 Latest deploy log https://app.netlify.com/projects/rspress-v2/deploys/686f6b84d0e1480008e342ed
😎 Deploy Preview https://deploy-preview-2363--rspress-v2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

This comment was marked as outdated.

@JounQin JounQin force-pushed the fix/dynamic-toc branch 2 times, most recently from f363a87 to ed5121a Compare July 10, 2025 07:23
@JounQin JounQin requested a review from Copilot July 10, 2025 07:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the dynamic table of contents by detecting heading mutations within child nodes and refactors the mutation observer loops to for…of. It also adds required type definitions and an end-to-end fixture to verify dynamic updates.

  • Refactor useDynamicToc to detect added/removed heading nodes (including children) using labeled for…of loops.
  • Add @types/web to support web types in the theme package.
  • Introduce a new E2E fixture (dynamic-toc) with config, sample component, MDX doc, and Playwright test.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/theme-default/src/components/Aside/useDynamicToc.ts Refactored MutationObserver loops, unified added/removed handling, and child-node checks.
packages/theme-default/package.json Added @types/web dependency for web-type definitions.
e2e/fixtures/dynamic-toc/rspress.config.ts Added Rspress fixture config file.
e2e/fixtures/dynamic-toc/package.json Added fixture package.json with dependencies.
e2e/fixtures/dynamic-toc/index.test.ts Added Playwright test for dynamic TOC updates.
e2e/fixtures/dynamic-toc/doc/index.mdx Sample MDX page using the Term component.
e2e/fixtures/dynamic-toc/components/Term.tsx Sample React component that injects dynamic content.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)

packages/theme-default/src/components/Aside/useDynamicToc.ts:100

  • [nitpick] The loop variable nodes is ambiguous; consider renaming it to nodeList or nodesCollection for clearer intent.
          for (const nodes of [mutation.addedNodes, mutation.removedNodes]) {

e2e/fixtures/dynamic-toc/index.test.ts:22

  • [nitpick] This test covers addition of headings but not removal. Consider adding a scenario where the component unmounts or headings are removed to verify the TOC updates correctly for removedNodes.
  test('Index page', async ({ page }) => {

Comment thread packages/theme-default/src/components/Aside/useDynamicToc.ts Outdated
Comment thread e2e/fixtures/dynamic-toc/index.test.ts
Comment thread e2e/fixtures/dynamic-toc/index.test.ts
@SoonIter SoonIter merged commit 3443ee5 into main Jul 10, 2025
11 checks passed
@SoonIter SoonIter deleted the fix/dynamic-toc branch July 10, 2025 08:01
@SoonIter
Copy link
Copy Markdown
Member

thanks❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working change: fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

It seems this only supports static TOC on the first render, for real dynamic TOC generation, MutationObserver could be considered.

3 participants