Skip to content

fix(Funnel): prevent NaN coordinate layout crash when all values are zero#7184

Merged
PavelVanecek merged 1 commit into
recharts:mainfrom
Mridul012:fix/funnel-zero-division
Apr 4, 2026
Merged

fix(Funnel): prevent NaN coordinate layout crash when all values are zero#7184
PavelVanecek merged 1 commit into
recharts:mainfrom
Mridul012:fix/funnel-zero-division

Conversation

@Mridul012

@Mridul012 Mridul012 commented Mar 29, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes a division-by-zero issue in Funnel when all values are 0, which caused NaN coordinates and broke rendering.

Fix

  • Guarded calculations for x, upperWidth, and lowerWidth when maxValue === 0
  • Fallback: x = offsetX, widths = 0

Tests

Added regression test to ensure:

  • No NaN values
  • Correct x and zero widths

Impact

No effect on normal datasets; prevents crashes for all-zero data.

Summary by CodeRabbit

  • Bug Fixes

    • Resolved issue where funnel chart would produce invalid calculations (NaN) when all input data values are zero. Chart now renders correctly in this edge case.

  • Tests

    • Added test coverage for funnel chart rendering with zero-value data.

@coderabbitai

coderabbitai Bot commented Mar 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1fa5482f-869b-4262-991e-4fa5e15bff10

📥 Commits

Reviewing files that changed from the base of the PR and between fc96239 and 1754d94.

📒 Files selected for processing (2)
  • src/cartesian/Funnel.tsx
  • test/cartesian/Funnel.spec.tsx

Walkthrough

The funnel trapezoid computation now guards against zero maxValue by returning default values (offsetX for x position, 0 for widths) instead of performing division operations that would produce NaN or Infinity when maxValue === 0. A corresponding test validates this behavior.

Changes

Cohort / File(s) Summary
Funnel NaN guard
src/cartesian/Funnel.tsx		 Added zero-value guards in computeFunnelTrapezoids: x returns offsetX when maxValue === 0; upperWidth and lowerWidth return 0 when maxValue === 0, otherwise use original ratio-based calculations.
Funnel test coverage
test/cartesian/Funnel.spec.tsx		 Added new test should not produce NaN when all data values are 0 that verifies computeFunnelTrapezoids returns finite values (not NaN/Infinity) when input data contains all zero values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description covers the key aspects (summary, fix, tests, impact) but is missing several required template sections like Related Issue, Motivation and Context, How Has This Been Tested details, and the Types of changes checklist. Complete the description by filling in the missing template sections, particularly adding the related issue link, detailed testing approach, and checking the appropriate boxes in the Types of changes section.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing a NaN crash in the Funnel component when all values are zero, which is the core objective of this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Mar 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.06%. Comparing base (fc96239) to head (1754d94).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #7184   +/-   ##
=======================================
  Coverage   89.06%   89.06%           
=======================================
  Files         539      539           
  Lines       41011    41011           
  Branches     5553     5552    -1     
=======================================
  Hits        36527    36527           
  Misses       4476     4476           
  Partials        8        8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codecov

codecov Bot commented Mar 30, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 231 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.3MB 69 bytes (0.01%) ⬆️
recharts/bundle-es6 1.13MB 69 bytes (0.01%) ⬆️
recharts/bundle-umd 547.92kB 24 bytes (0.0%) ⬆️
recharts/bundle-treeshaking-cartesian 644.16kB 69 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-treeshaking-cartesian

Assets Changed:

Asset Name Size Change Total Size Change (%)
bundle.js 69 bytes 644.16kB 0.01%
view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
cartesian/Funnel.js 69 bytes 19.1kB 0.36%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
cartesian/Funnel.js 69 bytes 17.61kB 0.39%
view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
Recharts.js 24 bytes 547.92kB 0.0%

@Mridul012

Mridul012 commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, Please let me know if any further changes are needed.

@PavelVanecek
PavelVanecek merged commit dbfca70 into recharts:main Apr 4, 2026
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ckifer ckifer ckifer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Mridul012 @ckifer @PavelVanecek