fix(docs): reinitialize TOC on route change #4854
Conversation
lakshya-byte
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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
📝 WalkthroughWalkthroughA React key prop is added to the TOC component in DocsLayout.tsx, set to Changes
Estimated Code Review Effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4854--asyncapi-website.netlify.app/ |
![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
🧹 Nitpick comments (1)
components/layout/DocsLayout.tsx (1)
146-146: Consider usingrouter.pathnameinstead ofrouter.asPathfor more targeted remounting.While
router.asPathworks correctly, it includes query parameters and hash fragments, causing the TOC to remount even for hash-only navigation within the same page (e.g., clicking TOC links that only change#section1→#section2). Since ScrollSpy tracks scroll position automatically, remounting on hash-only changes may be unnecessary.Using
router.pathnamewould limit remounts to actual page changes, reducing unnecessary reinitializations while still fixing the cross-page navigation bug.🔎 Alternative implementation
- key={router.asPath} + key={router.pathname}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/layout/DocsLayout.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
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.
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
📚 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/layout/DocsLayout.tsx
📚 Learning: 2024-10-11T11:32:30.226Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
Applied to files:
components/layout/DocsLayout.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/layout/DocsLayout.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/layout/DocsLayout.tsx
🪛 GitHub Actions: PR testing - if Node project
components/layout/DocsLayout.tsx
[error] 146-146: Prettier: Replace ·=· with =
⏰ 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). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
|
|
||
| <div className={`xl:flex ${post.toc && post.toc.length ? 'xl:flex-row-reverse' : ''}`}> | ||
| <TOC | ||
| key = {router.asPath} |
There was a problem hiding this comment.
Fix formatting: Remove spaces around the equals sign.
The spacing violates Prettier rules and blocks the pipeline.
🔎 Proposed fix
- key = {router.asPath}
+ key={router.asPath}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key = {router.asPath} | |
| key={router.asPath} |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 146-146: Prettier: Replace ·=· with =
🤖 Prompt for AI Agents
In components/layout/DocsLayout.tsx around line 146, the JSX prop assignment
"key = {router.asPath}" has extra spaces around the equals sign; remove the
spaces so it reads "key={router.asPath}" to satisfy Prettier and unblock the
pipeline.
|
@lakshya-byte pipeline is failing. have a look on that. Also please make sure to attach testing evidence from the temporary netlify deployed link |
3 participants
Description
Fixes #4845
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.