fix: restore TOC active section highlighting after client-side navigation #4919
Conversation
KariraLakshya
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughSet up Next.js router inside the TOC component and keyed the Scrollspy instance to Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-12-29T14:21:43.887ZApplied to files:
📚 Learning: 2024-10-11T07:38:35.745ZApplied to files:
📚 Learning: 2024-10-11T10:46:58.882ZApplied to files:
📚 Learning: 2025-01-18T08:44:43.614ZApplied to files:
📚 Learning: 2025-12-29T14:21:28.216ZApplied to files:
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
KariraLakshya
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/TOC.tsx (1)
90-97: Fix formatting: remove spaces around=in the key prop.The key prop has a formatting issue that's blocking the pipeline. Remove the spaces around the
=sign.🔧 Proposed fix for formatting
<div className={`${!open && 'hidden'} ${cssBreakingPoint === 'xl' ? 'xl:block' : 'lg:block'}`}> <Scrollspy - key = {router.asPath} + key={router.asPath} items={tocItems.map((item) => (item.slug ? item.slug : item.slugWithATag))} currentClassName='text-primary-500 font-bold' componentTag='div'
Solution approach is correct.
Using
router.asPathas the key effectively solves the issue by forcing Scrollspy to remount when navigating between pages. This resets its internal state and scroll listeners, restoring the active section highlighting functionality.
🤖 Fix all issues with AI agents
In @components/TOC.tsx:
- Around line 1-6: Imports at the top of the file (React, useState, Scrollspy,
twMerge, useRouter, ArrowRight) are not sorted per the
simple-import-sort/imports rule; run the auto-fix or reorder them to satisfy
ESLint: either run the project's formatter (npm run format) or run ESLint with
--fix for this file (npx eslint --fix) to automatically sort the imports, or
manually reorder the import lines so grouped/aliased imports follow the
simple-import-sort ordering.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/TOC.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
📚 Learning: 2024-10-11T10:46:58.882Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
Applied to files:
components/TOC.tsx
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/TOC.tsx
🪛 GitHub Actions: PR testing - if Node project
components/TOC.tsx
[error] 1-1: ESLint/Prettier: Run autofix to sort imports. Rule: simple-import-sort/imports
[error] 91-91: Prettier formatting issue. Replace key = with key=
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: cypress-run
- GitHub Check: Lighthouse CI
🔇 Additional comments (1)
components/TOC.tsx (1)
29-29: LGTM! Router instance correctly initialized.The router is properly instantiated using the
useRouterhook, which is the standard pattern for accessing route information in Next.js 15.
| import React, { useState } from 'react'; | ||
| import Scrollspy from 'react-scrollspy'; | ||
| import { twMerge } from 'tailwind-merge'; | ||
|
|
||
| import { useRouter } from 'next/router'; | ||
| import ArrowRight from './icons/ArrowRight'; | ||
|
|
There was a problem hiding this comment.
Fix import sorting to pass ESLint.
The pipeline reports that imports need to be sorted according to the simple-import-sort/imports rule. Run the autofix command to resolve this.
Run the following command to fix import sorting automatically:
npm run formator
npx eslint --fix components/TOC.tsx🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 1-1: ESLint/Prettier: Run autofix to sort imports. Rule: simple-import-sort/imports
🤖 Prompt for AI Agents
In @components/TOC.tsx around lines 1 - 6, Imports at the top of the file
(React, useState, Scrollspy, twMerge, useRouter, ArrowRight) are not sorted per
the simple-import-sort/imports rule; run the auto-fix or reorder them to satisfy
ESLint: either run the project's formatter (npm run format) or run ESLint with
--fix for this file (npx eslint --fix) to automatically sort the imports, or
manually reorder the import lines so grouped/aliased imports follow the
simple-import-sort ordering.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4919--asyncapi-website.netlify.app/ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4919 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 796 796
Branches 146 146
=========================================
Hits 796 796 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/TOC.tsx (1)
92-92: Consider usingrouter.pathnameto avoid unnecessary Scrollspy remounts on hash changes.The fix correctly forces Scrollspy to remount on route changes. However,
router.asPathincludes the hash fragment, which means clicking TOC links that only change the hash (e.g., from#section1to#section2) will trigger remounts even though the underlying page hasn't changed.Since the issue focuses on navigating between docs pages, use
router.pathnameinstead, which excludes query parameters and hashes:Suggested change
- key={router.asPath} + key={router.pathname}This limits remounts to actual page navigation, avoiding unnecessary disruption during same-page scrolling.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/TOC.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
📚 Learning: 2025-12-29T14:21:43.887Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:43.887Z
Learning: In the Filter component (components/navigation/Filter.tsx), the useEffect hooks are intentionally designed to react only to route/URL changes, not to prop changes (data, checks, onFilter). The omitted dependencies are by design.
Applied to files:
components/TOC.tsx
📚 Learning: 2024-10-11T07:38:35.745Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Applied to files:
components/TOC.tsx
📚 Learning: 2024-10-11T10:46:58.882Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
Applied to files:
components/TOC.tsx
📚 Learning: 2025-01-18T08:44:43.614Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Applied to files:
components/TOC.tsx
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/TOC.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: cypress-run
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
components/TOC.tsx (2)
1-1: LGTM! Import is compatible with Next.js 15.The
useRouterimport fromnext/routeris correct for the Pages Router, which Next.js 15 continues to support with backward compatibility.
30-30: LGTM! Proper hook usage.The router is correctly initialized at the top level of the component, following React's Rules of Hooks.
|
Update: @princerajpoot20 all the checks are passed and the PR is ready for Review |
2 participants
Description
This PR fixes an issue where the “On this page” Table of Contents (TOC) active section highlight stops working after navigating between documentation pages using client-side routing.
The TOC uses
react-scrollspy, which does not automatically reset when the route changes. As a result, it continues to track headings from the previously visited page, causing the active section highlight to break until a full page reload.What was Changed
useRouterto detect route changesScrollspycomponent withrouter.asPathto force a remount on navigationWhy this Works
Changing the
keyforces React to recreate theScrollspycomponent whenever the route changes. This resets its internal state and restores correct active section tracking without introducing custom scroll logic or breaking existing behavior.Related issue(s)
Fixes #4845
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.