fix(ResponsiveContainer): Fix erroneous console warning on init#7174
Conversation
WalkthroughThe changes fix an issue where ResponsiveContainer logs incorrect warning messages with negative dimensions during initial render. The warning condition in SizeDetectorContainer was refined to also check if measured containerWidth or containerHeight is negative. A test case was added to verify no warning is logged when dimensions haven't been measured yet. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Fixes an incorrect dev warning emitted by ResponsiveContainer during initial mount (before the container has been measured), aligning behavior with expected responsive sizing in Recharts.
Changes:
- Suppress the “width/height should be greater than 0” warning while the container dimensions are still in the initial
-1/-1sentinel state. - Add a regression test ensuring no warning is logged on initial render when using
%dimensions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/component/ResponsiveContainer.tsx |
Adjusts the warning condition to avoid logging during the pre-measurement sentinel size state. |
test/component/ResponsiveContainer.spec.tsx |
Adds a test asserting no console warning occurs on initial render before measurement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| containerWidth < 0 || | ||
| containerHeight < 0 || | ||
| (calculatedWidth != null && calculatedWidth > 0) || | ||
| (calculatedHeight != null && calculatedHeight > 0), |
There was a problem hiding this comment.
The warning guard currently treats the size as “OK” when either calculatedWidth or calculatedHeight is > 0 (plus the new negative-initial-dimension bypass). This means cases like containerWidth=0 with containerHeight>0 won’t emit the warning even though the chart can’t render (both dimensions must be >0 for ResponsiveContainerContextProvider to render children). Consider changing the condition to require both calculatedWidth and calculatedHeight to be >0 once the container has been measured, while still skipping the warning for the initial (-1/-1) sentinel dimensions.
| containerWidth < 0 || | |
| containerHeight < 0 || | |
| (calculatedWidth != null && calculatedWidth > 0) || | |
| (calculatedHeight != null && calculatedHeight > 0), | |
| (containerWidth < 0 && containerHeight < 0) || | |
| (calculatedWidth != null && | |
| calculatedWidth > 0 && | |
| calculatedHeight != null && | |
| calculatedHeight > 0), |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/component/ResponsiveContainer.spec.tsx`:
- Around line 397-407: The test 'should not warn on initial render before
dimensions are measured' is missing fake-timer setup—wrap the test with
vi.useFakeTimers() at the start (or ensure the suite-level setup covers it) and
call vi.runOnlyPendingTimers() immediately after the render of
ResponsiveContainer/DimensionSpy to flush timers; update the test to call
vi.useFakeTimers() before calling render and vi.runOnlyPendingTimers() after
render so consoleWarnSpy assertion runs against the expected timer state.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6888c4ba-750c-4f07-976f-a10f896d4ab0
📒 Files selected for processing (2)
src/component/ResponsiveContainer.tsxtest/component/ResponsiveContainer.spec.tsx
| it('should not warn on initial render before dimensions are measured', () => { | ||
| mockGetBoundingClientRect({ width: 400, height: 200 }); | ||
|
|
||
| render( | ||
| <ResponsiveContainer width="100%" height="100%"> | ||
| <DimensionSpy /> | ||
| </ResponsiveContainer>, | ||
| ); | ||
|
|
||
| expect(consoleWarnSpy).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 "should not warn on initial render before dimensions are measured|vi\\.useFakeTimers\\(|vi\\.runOnlyPendingTimers\\(" test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 403
🏁 Script executed:
#!/bin/bash
# Get the full test body and context around lines 397-407
sed -n '397,415p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 680
🏁 Script executed:
#!/bin/bash
# Check if this file uses createSelectorTestCase anywhere and how tests are generally structured
head -50 test/component/ResponsiveContainer.spec.tsx | grep -E "import|createSelectorTestCase"
tail -20 test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 1079
🏁 Script executed:
#!/bin/bash
# Get a broader view of test patterns - check multiple tests to see the timer pattern
grep -n "it('should" test/component/ResponsiveContainer.spec.tsx | head -15Repository: recharts/recharts
Length of output: 1255
🏁 Script executed:
#!/bin/bash
# Check how many tests use vi.useFakeTimers() vs don't use it
echo "Tests with vi.useFakeTimers():"
grep -c "vi\.useFakeTimers()" test/component/ResponsiveContainer.spec.tsx
echo "---"
echo "Total it() tests:"
grep -c "it('should" test/component/ResponsiveContainer.spec.tsx
echo "---"
# Get a few more test examples to understand the pattern
sed -n '161,175p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 588
🏁 Script executed:
#!/bin/bash
# Check tests around line 350 to see if they use timers
sed -n '350,365p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 730
🏁 Script executed:
#!/bin/bash
# Check the test at line 387 to see if it uses timers
sed -n '387,400p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 677
🏁 Script executed:
#!/bin/bash
# Check the "should resize when debounced" test to see the exact pattern used
sed -n '185,210p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Check if there's afterEach cleanup for timers in this test file
grep -n "afterEach\|afterAll\|useRealTimers" test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Get the complete test including any cleanup
sed -n '185,227p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 1561
🏁 Script executed:
#!/bin/bash
# Check if there's a beforeEach or afterEach that might handle timer setup/cleanup
sed -n '1,100p' test/component/ResponsiveContainer.spec.tsx | grep -A5 -B5 "beforeEach\|afterEach\|beforeAll\|afterAll"Repository: recharts/recharts
Length of output: 472
🏁 Script executed:
#!/bin/bash
# Check if other test files in the same directory use vi.useRealTimers() cleanup
grep -r "vi\.useRealTimers()" test/component/ | head -5Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check how the "should resize when debounced" test ends - does it have vi.useRealTimers() call?
sed -n '185,230p' test/component/ResponsiveContainer.spec.tsx | tail -5Repository: recharts/recharts
Length of output: 257
🏁 Script executed:
#!/bin/bash
# Double-check: show the exact end of the debounced test to confirm no cleanup
sed -n '220,228p' test/component/ResponsiveContainer.spec.tsxRepository: recharts/recharts
Length of output: 319
Add timer setup to align with test requirements.
The new test is missing required fake timer setup. Per the test guidelines, vi.useFakeTimers() must be used in all tests, and vi.runOnlyPendingTimers() must be called after renders when not using the createSelectorTestCase helper.
✅ Suggested patch
it('should not warn on initial render before dimensions are measured', () => {
+ vi.useFakeTimers();
mockGetBoundingClientRect({ width: 400, height: 200 });
render(
<ResponsiveContainer width="100%" height="100%">
<DimensionSpy />
</ResponsiveContainer>,
);
+ vi.runOnlyPendingTimers();
expect(consoleWarnSpy).not.toHaveBeenCalled();
});📝 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.
| it('should not warn on initial render before dimensions are measured', () => { | |
| mockGetBoundingClientRect({ width: 400, height: 200 }); | |
| render( | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <DimensionSpy /> | |
| </ResponsiveContainer>, | |
| ); | |
| expect(consoleWarnSpy).not.toHaveBeenCalled(); | |
| }); | |
| it('should not warn on initial render before dimensions are measured', () => { | |
| vi.useFakeTimers(); | |
| mockGetBoundingClientRect({ width: 400, height: 200 }); | |
| render( | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <DimensionSpy /> | |
| </ResponsiveContainer>, | |
| ); | |
| vi.runOnlyPendingTimers(); | |
| expect(consoleWarnSpy).not.toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/component/ResponsiveContainer.spec.tsx` around lines 397 - 407, The test
'should not warn on initial render before dimensions are measured' is missing
fake-timer setup—wrap the test with vi.useFakeTimers() at the start (or ensure
the suite-level setup covers it) and call vi.runOnlyPendingTimers() immediately
after the render of ResponsiveContainer/DimensionSpy to flush timers; update the
test to call vi.useFakeTimers() before calling render and
vi.runOnlyPendingTimers() after render so consoleWarnSpy assertion runs against
the expected timer state.
There was a problem hiding this comment.
The fake timers are already included in the global setup file.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find vitest config and global setup files
fd -e ts -e js -e mts "vitest.config|vitest.setup|setup" --exclude node_modules | head -20
echo "---"
# Check vitest config for setupFiles
rg -n "setupFiles|globalSetup|fakeTimers|useFakeTimers" vitest.config.ts vitest.config.mts 2>/dev/null || rg -rn "setupFiles|globalSetup|fakeTimers|useFakeTimers" --include="vitest.config*" 2>/dev/null
Length of output: 338
🏁 Script executed:
#!/bin/bash
cat test/vitest.setup.ts
echo "---"
rg -n "fakeTimers|useFakeTimers" vitest.config.mts
Length of output: 1614
@PavelVanecek, you're absolutely right — I can confirm that test/vitest.setup.ts calls vi.useFakeTimers() at the module level, which is picked up via setupFiles in vitest.config.mts and applies globally to all tests. My suggestion to add it per-test was incorrect. Apologies for the noise!
I'll update the learning to reflect that fake timers are configured globally rather than requiring per-test setup.
🐇 (づ｡◕‿‿◕｡)づ noted for future reviews!)
✏️ Learnings added
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 7174
File: test/component/ResponsiveContainer.spec.tsx:397-407
Timestamp: 2026-03-27T05:32:42.402Z
Learning: In recharts/recharts, `vi.useFakeTimers()` is called globally in `test/vitest.setup.ts` (registered as a `setupFiles` entry in `vitest.config.mts`). Individual tests do NOT need to call `vi.useFakeTimers()` themselves — it is already active for the entire test suite. Tests that need to advance time should still call `vi.runOnlyPendingTimers()` where appropriate after renders.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Mock `getBoundingClientRect` in tests using the helper function provided in `test/helper/MockGetBoundingClientRect.ts`
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Call `vi.runOnlyPendingTimers()` to advance timers after renders when not using `createSelectorTestCase` helper, and avoid `vi.runAllTimers()` to prevent infinite loops
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })` or the `userEventSetup` helper function from `test/helper/userEventSetup.ts` when creating userEvent instances
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : When testing tooltips on hover, use `vi.runOnlyPendingTimers()` after each `userEvent.hover()` call or use the `showTooltip` helper function from `tooltipTestHelpers.ts` to account for requestAnimationFrame delays
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use the `expectLastCalledWith` helper function instead of `expect(spy).toHaveBeenLastCalledWith(...)` for better typing and autocompletion
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-01-31T13:38:06.811Z
Learning: Applies to test/component/**/*.spec.{ts,tsx} : Use React Testing Library for testing component interactions and rendering behavior
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-01-31T13:38:06.811Z
Learning: Ensure test suite passes with `npm run test` before submitting a pull request
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6783
File: test/util/ChartUtils.spec.tsx:15-16
Timestamp: 2025-12-16T08:12:06.809Z
Learning: In tests (files under any test directory, e.g., test/, __tests__/, etc.), allow importing internal module paths (e.g., ../../src/...) and do not require imports to use the public API entry point (src/index.ts). The public API import restriction should apply only to non-test code. During reviews, accept internal imports in test files and enforce the public API pattern only for non-test code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7174 +/- ##
=======================================
Coverage 89.68% 89.68%
=======================================
Files 537 537
Lines 40674 40677 +3
Branches 5547 5549 +2
=======================================
+ Hits 36477 36480 +3
Misses 4189 4189
Partials 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 100 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
|
Description
Fix issue with ResponsiveContainer where warning is output before rendering non-default values.
This detects presence of output values and prevents warning. This could also be fixed by using something like
undefinedbut I'm proceeding with-1precedent and just making it go awayRelated Issue
Fixes: #6716
Motivation and Context
Removes warning
How Has This Been Tested?
Wrote test detecting console warning which fails before this change, passes after the fix.
I also checked before/after state in story book and my browser's console
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit
Bug Fixes
Tests