Conversation
WalkthroughAdds locale-aware redirects (both runtime and prerendered), renames SankeyChart → Sankey in docs/navigation, extracts navigation helpers into Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Router
participant Redirects
participant PreRenderer
participant Renderer
rect rgb(220, 240, 220)
note over Router,Redirects: Runtime routing with redirects
Browser->>Router: Request /api/SankeyChart/
Router->>Redirects: lookup('/api/SankeyChart/')
Redirects-->>Router: '/api/Sankey/' (match)
Router->>Browser: Navigate to /api/Sankey/ (Navigate replace)
end
rect rgb(200, 220, 255)
note over PreRenderer,Redirects: Prerendering loop
PreRenderer->>PreRenderer: iterate routes
alt route has redirect
PreRenderer->>Redirects: get target
Redirects-->>PreRenderer: targetUrl
PreRenderer->>PreRenderer: createRedirectHtml(targetUrl)
PreRenderer->>Disk: write redirect HTML
else normal route
PreRenderer->>Renderer: render(route)
Renderer-->>Disk: write rendered HTML
end
end
rect rgb(240, 220, 220)
note over Browser,Router: Nav item detection
Browser->>Router: Navigate to /guide/
Router->>Router: normalizePathnameToLocale('/guide/') -> '/en-US/guide/'
Router->>Router: useCurrentNavItem() -> active NavItem
Router-->>Browser: render with active nav highlight
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)www/**📄 CodeRabbit inference engine (DEVELOPING.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-25T07:36:02.229ZApplied to files:
🧬 Code graph analysis (1)www/src/docs/api/index.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
www/test/__snapshots__/navigation.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
www/scripts/prerender.tsx(3 hunks)www/src/components/Shared/SidebarNav/index.tsx(1 hunks)www/src/docs/api/Sankey.ts(1 hunks)www/src/docs/api/index.ts(2 hunks)www/src/docs/apiCates.ts(1 hunks)www/src/navigation.data.ts(1 hunks)www/src/navigation.ts(2 hunks)www/src/routes/index.tsx(1 hunks)www/src/routes/redirects.ts(1 hunks)www/test/navigation.spec.ts(2 hunks)www/test/redirects.spec.ts(1 hunks)www/test/sitemap.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
www/**
📄 CodeRabbit inference engine (DEVELOPING.md)
Use the www directory to add and commit examples for the documentation website (recharts.github.io)
Files:
www/src/routes/index.tsxwww/src/navigation.data.tswww/src/docs/api/index.tswww/test/redirects.spec.tswww/src/routes/redirects.tswww/src/docs/api/Sankey.tswww/src/docs/apiCates.tswww/src/navigation.tswww/scripts/prerender.tsxwww/test/navigation.spec.tswww/src/components/Shared/SidebarNav/index.tsxwww/test/sitemap.spec.ts
{test,www/test}/**
📄 CodeRabbit inference engine (DEVELOPING.md)
Place unit tests in the test directory; some tests may also live in www/test
Files:
www/test/redirects.spec.tswww/test/navigation.spec.tswww/test/sitemap.spec.ts
🧠 Learnings (3)
📚 Learning: 2025-10-25T07:36:02.229Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.229Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility
Applied to files:
www/src/docs/api/index.ts
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/vitest.setup.ts : Ensure vi.useFakeTimers() is configured in the Vitest setup file
Applied to files:
www/test/redirects.spec.ts
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Write unit tests using Vitest and React Testing Library
Applied to files:
www/test/redirects.spec.tswww/test/navigation.spec.ts
🧬 Code graph analysis (9)
www/src/routes/index.tsx (1)
www/src/routes/redirects.ts (1)
redirects(18-21)
www/src/docs/api/index.ts (1)
www/src/docs/api/Sankey.ts (1)
SankeyAPI(3-226)
www/test/redirects.spec.ts (1)
www/src/routes/redirects.ts (1)
redirects(18-21)
www/src/routes/redirects.ts (1)
www/src/locale/index.ts (1)
supportedLocales(11-11)
www/src/docs/api/Sankey.ts (1)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)
www/src/navigation.ts (2)
www/src/locale/index.ts (1)
supportedLocales(11-11)www/src/utils/LocaleUtils.ts (1)
defaultLocale(6-6)
www/scripts/prerender.tsx (2)
www/src/routes/redirects.ts (1)
redirects(18-21)www/src/entry-server.tsx (1)
render(7-11)
www/test/navigation.spec.ts (1)
www/src/navigation.ts (1)
normalizePathnameToLocale(138-147)
www/test/sitemap.spec.ts (1)
www/src/routes/redirects.ts (2)
redirects(18-21)redirectsWithoutLocale(3-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
www/src/docs/apiCates.ts (1)
15-15: LGTM!The rename from 'SankeyChart' to 'Sankey' in the charts category is consistent with the exported component name and aligns with the PR objectives.
www/src/docs/api/index.ts (1)
8-8: LGTM!The addition of
SankeyAPIwhile retainingSankeyChartAPImaintains backward compatibility. The redirect system handles URL mapping, making this a safe transition.Also applies to: 65-65
www/src/navigation.data.ts (1)
12-13: Verify if both 'Sankey' and 'SankeyChart' should appear in navigation.Both component names are now in the
apiComponentsarray, which will generate navigation items and routes for both. This means users might see duplicate entries in the navigation. Consider whether only 'Sankey' should be present here, with redirects handling the old 'SankeyChart' URLs transparently.www/src/routes/index.tsx (1)
1-1: LGTM!The redirect implementation is well-structured. Using
Navigatewithreplaceprop prevents back-button issues, and positioning the redirects before other routes ensures they're matched first.Also applies to: 4-4, 9-12
www/test/redirects.spec.ts (1)
1-14: LGTM!The test provides good coverage of the redirect mappings, validating both locale-prefixed and non-prefixed URLs. The inline snapshot approach will catch any unexpected changes to the redirect configuration.
www/test/navigation.spec.ts (1)
2-2: LGTM!The test suite for
normalizePathnameToLocaleprovides good coverage, validating both scenarios where a locale is present or absent in the pathname.Also applies to: 12-22
www/test/sitemap.spec.ts (1)
5-5: LGTM!The test correctly includes redirect URLs in the sitemap validation by merging
redirectsWithoutLocalekeys with the navigation URLs. This ensures redirect routes are accounted for in the site structure.Also applies to: 31-32
www/src/components/Shared/SidebarNav/index.tsx (1)
1-6: LGTM!The refactoring to use the centralized
useCurrentNavItemhook from the navigation module is a good improvement. This reduces code duplication and makes the navigation logic more maintainable.www/scripts/prerender.tsx (2)
21-38: LGTM!The redirect HTML implementation follows best practices with multiple redirect mechanisms (meta refresh, canonical link, JavaScript, and user-visible fallback).
85-94: LGTM!The conditional logic correctly handles both redirect and normal rendering paths, with appropriate logging for each case.
www/src/routes/redirects.ts (2)
3-5: LGTM!The base redirect mapping is clear and correctly defines the SankeyChart to Sankey redirect.
7-15: LGTM!The locale-aware redirect generation correctly prefixes both source and target paths with each supported locale.
www/src/navigation.ts (2)
138-147: LGTM!The normalization logic correctly detects existing locale prefixes and falls back to prepending the default locale when needed.
149-154: LGTM!The hook correctly normalizes the pathname and finds the matching navigation item. Using
includes()is appropriate for matching parent navigation items.
| { | ||
| name: 'dataKey', | ||
| type: 'String | Number | Function', | ||
| defaultVal: 'value', | ||
| isOptional: true, | ||
| desc: { | ||
| 'en-US': 'The key of a group of data which should be unique in a SankeyChart.', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Update component name reference.
The description refers to "SankeyChart" but the component has been renamed to "Sankey". Update for consistency.
Apply this diff:
desc: {
- 'en-US': 'The key of a group of data which should be unique in a SankeyChart.',
+ 'en-US': 'The key of a group of data which should be unique in a Sankey.',
},📝 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.
| { | |
| name: 'dataKey', | |
| type: 'String | Number | Function', | |
| defaultVal: 'value', | |
| isOptional: true, | |
| desc: { | |
| 'en-US': 'The key of a group of data which should be unique in a SankeyChart.', | |
| }, | |
| }, | |
| { | |
| name: 'dataKey', | |
| type: 'String | Number | Function', | |
| defaultVal: 'value', | |
| isOptional: true, | |
| desc: { | |
| 'en-US': 'The key of a group of data which should be unique in a Sankey.', | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In www/src/docs/api/Sankey.ts around lines 16 to 24, the prop description refers
to "SankeyChart" but the component is named "Sankey"; update the text to use
"Sankey" instead of "SankeyChart" so the docs match the component name. Replace
the phrase "SankeyChart" with "Sankey" in the desc field for the dataKey prop.
| { | ||
| name: 'sort', | ||
| type: 'Boolean', | ||
| defaultVal: 'true', | ||
| isOptional: true, | ||
| desc: { | ||
| 'en-US': 'Whether to sort the nodes on th y axis, or to display them as user-defined.', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Fix typo in description.
"on th y axis" should be "on the y axis".
Apply this diff:
desc: {
- 'en-US': 'Whether to sort the nodes on th y axis, or to display them as user-defined.',
+ 'en-US': 'Whether to sort the nodes on the y axis, or to display them as user-defined.',
},📝 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.
| { | |
| name: 'sort', | |
| type: 'Boolean', | |
| defaultVal: 'true', | |
| isOptional: true, | |
| desc: { | |
| 'en-US': 'Whether to sort the nodes on th y axis, or to display them as user-defined.', | |
| }, | |
| }, | |
| { | |
| name: 'sort', | |
| type: 'Boolean', | |
| defaultVal: 'true', | |
| isOptional: true, | |
| desc: { | |
| 'en-US': 'Whether to sort the nodes on the y axis, or to display them as user-defined.', | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In www/src/docs/api/Sankey.ts around lines 67 to 75, the description for the
'sort' prop has a typo ("on th y axis"); update the string to "on the y axis" so
the English description reads: "Whether to sort the nodes on the y axis, or to
display them as user-defined." Make the change only to the desc['en-US'] text
and preserve surrounding formatting and punctuation.
| { | ||
| name: 'linkCurvature', | ||
| type: 'Number', | ||
| defaultVal: '0.5', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The curvature of width', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Clarify description.
"The curvature of width" is unclear. This should likely be "The curvature of the link" or specify what aspect the curvature affects.
Consider updating the description to clarify what the curvature parameter controls.
🤖 Prompt for AI Agents
In www/src/docs/api/Sankey.ts around lines 102 to 110, the description "The
curvature of width" is unclear; change the desc to explicitly state what
linkCurvature controls (for example: "The curvature of the links between nodes"
or "Controls how curved the link paths are; 0 = straight lines, 1 = maximum
curvature"). Update the 'en-US' string to a concise, unambiguous sentence
describing the parameter's effect and acceptable value range or behavior.
| { | ||
| name: 'sourceX', | ||
| type: 'Number', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The source number of X-axis', | ||
| }, | ||
| }, | ||
| { | ||
| name: 'sourceY', | ||
| type: 'Number', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The source number of Y-axis', | ||
| }, | ||
| }, | ||
| { | ||
| name: 'sourceControlX', | ||
| type: 'Number', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The source control of X-axis', | ||
| }, | ||
| }, | ||
| { | ||
| name: 'targetControlX', | ||
| type: 'Number', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The target control of X-axis', | ||
| }, | ||
| }, | ||
| { | ||
| name: 'targetX', | ||
| type: 'Number', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The target of X-axis', | ||
| }, | ||
| }, | ||
| { | ||
| name: 'targetY', | ||
| type: 'Number', | ||
| isOptional: false, | ||
| desc: { | ||
| 'en-US': 'The target of Y-axis', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify coordinate props documentation.
The props sourceX, sourceY, sourceControlX, targetControlX, targetX, and targetY are marked as required (isOptional: false) with no default values. These appear to be calculated properties rather than user-provided configuration props. Please verify:
- Are these actually user-facing props that need to be documented?
- If they are internal/calculated properties, they should be removed from the API documentation.
- If they are user-facing, they likely need better descriptions and examples.
Run the following script to check if these props are part of the actual Sankey component's public API:
🏁 Script executed:
#!/bin/bash
# Description: Check Sankey component prop definitions
# Find the Sankey component implementation
fd -e tsx -e ts -e jsx -e js Sankey --exec rg -l "export.*Sankey" {} \; | head -5
# Search for prop definitions in the Sankey component
rg -A 10 "interface.*Sankey.*Props|type.*Sankey.*Props" --type ts --type tsxLength of output: 180
🏁 Script executed:
#!/bin/bash
# View the actual Sankey component implementation
echo "=== Sankey Component Implementation ==="
cat src/chart/Sankey.tsx | head -100
echo -e "\n=== Searching for Props interface in Sankey files ==="
# Use rg with proper type flags, searching for Props definitions
rg -n "Props\s*=|interface.*Props" src/chart/Sankey.tsx --context 5
echo -e "\n=== Searching for sourceX, sourceY in implementation ==="
rg -n "sourceX|sourceY" src/chart/Sankey.tsx --context 2
echo -e "\n=== Check the doc file structure around the coordinate props ==="
sed -n '140,230p' www/src/docs/api/Sankey.tsLength of output: 463
🏁 Script executed:
#!/bin/bash
# Discover repository structure
echo "=== Repository Root Structure ==="
ls -la | head -20
echo -e "\n=== Find all Sankey-related files ==="
fd Sankey
echo -e "\n=== Check if www directory exists ==="
ls -la www/ 2>/dev/null | head -10Length of output: 2659
🏁 Script executed:
#!/bin/bash
# View Sankey component Props definition
echo "=== Sankey.tsx - First 150 lines ==="
head -150 src/chart/Sankey.tsx
echo -e "\n=== Search for Props interface in Sankey.tsx ==="
rg -n "interface.*Props|type.*Props" src/chart/Sankey.tsx -A 15
echo -e "\n=== Search for sourceX and coordinate props in Sankey.tsx ==="
rg -n "sourceX|sourceY|sourceControlX|targetControlX|targetX|targetY" src/chart/Sankey.tsx -B 2 -A 2
echo -e "\n=== View documentation file context (lines 140-230) ==="
sed -n '140,230p' www/src/docs/api/Sankey.tsLength of output: 11491
Remove coordinate props from Sankey component API documentation.
These properties (sourceX, sourceY, sourceControlX, targetControlX, targetX, targetY) are part of the LinkProps interface, not the main Sankey component props. They are calculated internally and passed to custom link renderers—users never provide them directly to the Sankey component.
Lines 176-223 should be removed from the documentation. If custom link rendering needs to be documented, create a separate section documenting LinkProps with its coordinate properties and how they're used in custom link components.
🤖 Prompt for AI Agents
In www/src/docs/api/Sankey.ts around lines 176 to 223, remove the six coordinate
prop entries (sourceX, sourceY, sourceControlX, targetControlX, targetX,
targetY) from the Sankey component API docs because they belong to LinkProps and
are computed internally; then add (or update) a separate "LinkProps"
documentation section describing these coordinate properties and how they are
passed to custom link renderers (include types and brief usage example text) so
consumers know they are available when implementing custom links.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6560 +/- ##
=======================================
Coverage 91.27% 91.27%
=======================================
Files 489 490 +1
Lines 40848 40911 +63
Branches 4570 4581 +11
=======================================
+ Hits 37282 37341 +59
- Misses 3550 3554 +4
Partials 16 16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportBundle size has no change ✅ |
Change the URL and nav item so that the website doc matches what we export.
Discovered in #6552
Summary by CodeRabbit
New Features
Refactor
Docs
Tests