Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[charts] Piechart crashes when series is empty #16655

Closed
alexbalonperin opened this issue Feb 19, 2025 · 7 comments
Closed

[charts] Piechart crashes when series is empty #16655

alexbalonperin opened this issue Feb 19, 2025 · 7 comments
Assignees
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module!

Comments

@alexbalonperin
Copy link

alexbalonperin commented Feb 19, 2025

Steps to reproduce

I am skipping the live example because it's easy to reproduce the issue in a console or any js environment

Steps:

  1. Open your browser console
  2. Type: new Array(Math.max(...[]))
  3. Observe the following error: Uncaught RangeError: Invalid array length
  4. This is what happens when passing an empty series to the PieChart component at least on version 7.27.0 (see line 179 https://github.com/mui/mui-x/blob/v7.27.0/packages/x-charts/src/PieChart/PieChart.tsx#L179)

Current behavior

PieChart crashes when passing an empty series (e.g. []) as argument

Expected behavior

PieChart should show the noDataOverlay when passing an empty series as argument

Context

Show PieChart with dynamic data that may be empty if a set of filters leads to no data for example.

Your environment

npx @mui/envinfo
  System:
    OS: macOS 15.3.1
  Binaries:
    Node: 20.18.2 - ~/.nvm/versions/node/v20.18.2/bin/node
    npm: 10.8.2 - ~/.nvm/versions/node/v20.18.2/bin/npm
    pnpm: 9.15.4 - ~/.nvm/versions/node/v20.18.2/bin/pnpm
  Browsers:
    Chrome: 133.0.6943.126
  npmPackages:
    @mui/core-downloads-tracker:  6.1.0
    @mui/icons-material: 6.1.0 => 6.1.0
    @mui/material: 6.1.0 => 6.1.0
    @mui/private-theming:  6.1.0
    @mui/styled-engine:  6.1.0
    @mui/styled-engine-sc: ^6.1.0 => 6.1.0
    @mui/styles: ^6.1.0 => 6.1.0
    @mui/system:  6.1.0
    @mui/types:  7.2.16
    @mui/utils:  5.16.6
    @mui/x-charts: ^7.27.0 => 7.27.0
    @mui/x-charts-vendor:  7.20.0
    @mui/x-date-pickers: ^7.16.0 => 7.16.0
    @mui/x-internals:  7.16.0
    @mui/x-tree-view: ^7.16.0 => 7.16.0
    @types/react: ^18.3.12 => 18.3.12
    react: 18.2.0 => 18.2.0
    react-dom: 18.2.0 => 18.2.0
    styled-components: ^6.1.13 => 6.1.13
    typescript: ^4.9.5 => 4.9.5

Search keywords: charts piechart series empty

@alexbalonperin alexbalonperin added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 19, 2025
@github-actions github-actions bot added the component: charts This is the name of the generic UI component, not the React module! label Feb 19, 2025
@bernardobelchior
Copy link
Member

Hey, @alexbalonperin! Thank you for reporting and for finding the offending line. I've managed to reproduce the issue and I'm working on a fix.

@alexbalonperin
Copy link
Author

Thank you so much for the quick reply.
I was going to push a fix but I am struggling to run tests to confirm that the fix is working as expected.
Even when passing an empty series to the PieChart in PieChart.test.tsx, the test is still passing so I assume that the test is checking something else. I am not familiar with Mui internal test utils so I am sure that I am missing something obvious.
Anyway, thank you for helping with this.

@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 19, 2025
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 19, 2025

I am struggling to run tests to confirm that the fix is working as expected. Even when passing an empty series to the PieChart in PieChart.test.tsx, the test is still passing so I assume that the test is checking something else

@alexbalonperin What did you try to run those tests? I have added one with #16657 (comment). Hopefully next time, you will be able to make a PR on your own 😄. Normally, https://github.com/mui/mui-x/blob/master/CONTRIBUTING.md is up to date and is enough to test changes.
My best guess: you were on HEAD, so on v8 which doesn't have this bug.

@alexbalonperin
Copy link
Author

@oliviertassinari Thank you for pointing me at the CONTRIBUTING document.

  • I was actually on the v7.x branch. v8.x or master doesn't have the offending code as you mentioned which is how I figured that I was on master and switched to v7.x before running the tests
  • I was running the correct command pnpm test:unit but I didn't know about the --grep option so I just modified the original test:unit script directly in package.json and made a mistake. I used packages/x-charts/*.test.{js,ts,tsx} instead of packages/x-charts/**/*.test.{js,ts,tsx}. Silly me... 🤦

Anyway, thank you both for jumping in and writing a fix so quickly.

@bernardobelchior
Copy link
Member

bernardobelchior commented Feb 20, 2025

Hey, @alexbalonperin! The fix has been merged. It should be shipped as part of the next release for MUI X v7. Thank you for your contribution!

Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@alexbalonperin How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 21, 2025

Off-topic.

@michelengelen This makes me think of Colm's point. The first part of the bot comment is great

Image

but this part:

Image

feels out of place for a MIT issue. This feels Pro/Premium support issues. Not saying we have to remove it, but +1 on my end for a path where we remove this for community issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

4 participants