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

[AccordionSummary] false positive propType warning with disableGeneration #27363

Closed
2 tasks done
eahenke opened this issue Jul 19, 2021 · 3 comments
Closed
2 tasks done
Labels
bug 🐛 Something doesn't work component: accordion This is the name of the generic UI component, not the React module! v4.x

Comments

@eahenke
Copy link

eahenke commented Jul 19, 2021

When AccordionSummary is checking for usage of the deprecated classes.focused prop, the code calls indexOf on classes.focused without checking if that property is defined. If you don't supply classes.focused, this throws Failed prop type: Cannot read property 'indexOf' of undefined.

It appears to have been introduced in this change: 9c48d0b#diff-a3e45615b0516b905db6b26935126e30763f45b8901b2bee960adb5c3be2b997R147

A workaround is to always declare focused as an empty string in the classes prop (note: this does not work in Typescript, since that class was removed from the class keys), but ideally the checker should check if it's defined first.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

An error is thrown if you use AccordionSummary without explicitly passing classes.focused

Expected Behavior 🤔

No error should be thrown.

Steps to Reproduce 🕹

CodeSandbox: https://codesandbox.io/s/thirsty-ives-bwmqv

Steps:

  1. Use ServerStyleSheets with disableGeneration: true option
  2. Use AccordionSummary with a classes prop that does not include focused
  3. Receive propType warning Failed prop type: Cannot read property 'indexOf' of undefined

Context 🔦

This can break tests that check that components are being passed the correct props (for example, using the prop-types-throw library: https://www.npmjs.com/package/prop-types-throw)

Your Environment 🌎

`npx @material-ui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @material-ui/envinfo` goes here.
@eahenke eahenke added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 19, 2021
@eps1lon
Copy link
Member

eps1lon commented Jul 20, 2021

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

@eps1lon eps1lon added component: accordion This is the name of the generic UI component, not the React module! status: waiting for author Issue with insufficient information v4.x and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 20, 2021
@eahenke
Copy link
Author

eahenke commented Jul 20, 2021

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Thanks, updated with a CodeSandbox link and repro steps with some additional context.

@eps1lon eps1lon added bug 🐛 Something doesn't work and removed status: waiting for author Issue with insufficient information labels Jul 21, 2021
@eps1lon eps1lon changed the title AccordionSummary deprecated propType warning throws indexOf undefined error [AccordionSummary] false positive propType warning with disableGeneration Jul 21, 2021
@eps1lon
Copy link
Member

eps1lon commented Jul 26, 2021

Fixed in #27385

@eps1lon eps1lon closed this as completed Jul 26, 2021
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: accordion This is the name of the generic UI component, not the React module! v4.x
Projects
None yet
Development

No branches or pull requests

2 participants