fix: overview matching logic #158
Conversation
🦋 Changeset detectedLatest commit: 6095425 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe patch removes runtime href normalization from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant O as Overview.tsx
participant S as Sidebar Utils (_utils.ts)
participant R as @rspress/shared.normalizeHref
rect rgba(230,240,255,0.5)
note over O: Rendering links
U->>O: View overview
O->>O: Build link hrefs
note over O: New: use item.link directly
end
rect rgba(240,255,240,0.5)
note over S: Route matching
O->>S: findItemByRoutePath(routePath)
S->>R: normalizeHref(link, true)
R-->>S: normalizedLink
S-->>O: matched item
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: JounQin <admin@1stg.me>
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.changeset/new-keys-mix.md (1)
5-5: Clarify the release note for future readers.Adding a one-liner about what “overview matching logic” specifically fixes will help consumers scanning the changelog.
fix: overview matching logic + + - Centralize path normalization in utils via `@rspress/shared.normalizeHref` and remove redundant per-link normalization in `Overview`. Fixes mismatches between `/foo`, `/foo/`, and `/foo/index` during overview grouping.src/runtime/components/_utils.ts (2)
8-10: Minor: rename helper to reflect behavior.
removeIndexnow performs full normalization (not just trimming/index). Consider renaming tonormalizePath(or similar) to better mirror semantics. No functional change required.
64-69: Review remaining manual “/index” logic
The grep results reveal additional call sites that still rely on manual “/index” or regex-based trimming rather than usingremoveIndex/normalizeHref. To ensure consistent URL normalization across the codebase, please review and consider refactoring these occurrences:• src/theme.tsx (around line 40):
if (urlPath.endsWith('/index')) { … }• src/cli/export.ts (around line 185):
…foundSidebarItems.find(it => it.link?.endsWith('/index'))…You can replace these with calls to
removeIndex(...)(or its underlyingnormalizeHref) to unify behavior and avoid edge cases with trailing slashes.src/runtime/components/Overview.tsx (1)
241-241: Consider encoding header IDs when building hash links.If any header IDs contain characters needing encoding,
encodeURIComponent(header.id)would be safer. Rspress usually slugifies IDs, so this is a precautionary hardening.- href={`${item.link}#${header.id}`} + href={`${item.link}#${encodeURIComponent(header.id)}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/new-keys-mix.md(1 hunks)src/runtime/components/Overview.tsx(3 hunks)src/runtime/components/_utils.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/new-keys-mix.md
[grammar] ~5-~5: There might be a mistake here.
Context: ... patch --- fix: overview matching logic
(QB_NEW_EN)
🔇 Additional comments (4)
.changeset/new-keys-mix.md (1)
1-3: LGTM: patch bump is correct for a bug fix.Version scope matches the change impact (internal correctness, no API surface change).
src/runtime/components/_utils.ts (1)
6-6: Good move: reuse normalizeHref from the shared package.This consolidates normalization logic in one well-tested utility, reducing drift between components.
src/runtime/components/Overview.tsx (2)
6-6: Import cleanup is appropriate.Dropping the local normalize step aligns with centralizing normalization in utils and avoids redundant work at render time.
227-227: Confirm Link’s base‐path and normalized‐route handlingI verified that in
Overview.tsx(lines ~227) we passhref={item.link}directly, andLinkis imported from@rspress/core/themerather than using our localnormalizeHrefutility. Because we can’t inspect the theme’sLinkimplementation here, please double-check that:
Linkautomatically applies the site’s base path (e.g./docs/) when rendering internal routes- It does not re-normalize or duplicate already-normalized, site-relative paths
This will ensure no double or missing base segments on sites served from a non-root base.
Summary by CodeRabbit
Bug Fixes
Chores