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

The prop maxPropObjectKeys is marked as required in PropVal, but its value is undefined #1305

Closed
Stupidism opened this issue Jun 17, 2017 · 8 comments

Comments

@Stupidism
Copy link
Contributor

Stupidism commented Jun 17, 2017

My repo: https://github.com/Stupidism/stupid-rc-starter/tree/master/starter

warning.js?8a56:36 Warning: Failed prop type: The prop `maxPropObjectKeys` is marked as required in `PropVal`, but its value is `undefined`.
    in PropVal (created by PropVal)
    in PropVal (created by Props)
    in span (created by Props)
    in span (created by Props)
    in span (created by Props)
    in span (created by Props)
    in Props (created by Node)
    in div (created by Node)
    in div (created by Node)
    in Node (created by Story)
    in pre (created by Pre)
    in Pre (created by Story)
    in div (created by Story)
    in div (created by Story)
    in div (created by Story)
    in div (created by Story)

I looked at the code. There's no defaultProps here.

But maybe there should no be because, theoretically, all these props come from Story, which has defaultProps.


Found the real location: https://github.com/storybooks/storybook/blob/master/addons/info/src/components/PropVal.js#L64

@Stupidism
Copy link
Contributor Author

Temp fix in config.js:

// temp fix PropVal.propTypes
PropVal.propTypes = {
  ...PropVal.propTypes,
  maxPropObjectKeys: PropTypes.number,
  maxPropArrayLength: PropTypes.number,
  maxPropStringLength: PropTypes.number,
};

@tmeasday tmeasday added the bug label Jun 23, 2017
@tmeasday
Copy link
Member

Hi @Stupidism -- I don't quite understand this. How do I see the issue when running your reproduction?

@Stupidism
Copy link
Contributor Author

Stupidism commented Jun 23, 2017

clone repo
yarn install && npm start
these warnings are in console

I located the bug, too:
in PropVal.js

items[v${i}] = <PropVal val={val[name]} />;

@tmeasday
Copy link
Member

Ok, thanks I can see a problem, although not the one you reported about. How do I see the PropVal issue? (I see an issue for Props when browsing to the RenderCounter)

It would be great to create a simpler reproduction of the issue--perhaps a variation of the current cra-kitchen-sink project here: https://github.com/storybooks/storybook/blob/master/examples/cra-kitchen-sink/src/stories/index.js#L48

@Stupidism
Copy link
Contributor Author

Oh, because I fixed it temporarily by modifying the propTypes in this commit.

try git revert bf954db9a39694e549fb20daa85ff94576937b41 in my repo.

Temp fix in config.js:

@Li0liQ
Copy link
Contributor

Li0liQ commented Jun 28, 2017

The issue is reproduced by passing an array or an object as a property.
It was introduced in 70f7b51 by adding required props:

PropVal.propTypes = {
  val: PropTypes.any.isRequired, // eslint-disable-line
  maxPropObjectKeys: PropTypes.number.isRequired,
  maxPropArrayLength: PropTypes.number.isRequired,
  maxPropStringLength: PropTypes.number.isRequired,
};

The lines causing it to fail are:
https://github.com/storybooks/storybook/blob/master/addons/info/src/components/PropVal.js#L43
https://github.com/storybooks/storybook/blob/master/addons/info/src/components/PropVal.js#L64

Basically, in case of array/object elements the only prop being passed is val. Hence, validation errors.

Li0liQ added a commit to Li0liQ/storybook that referenced this issue Jun 28, 2017
@stale
Copy link

stale bot commented Nov 14, 2017

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks!

@stale stale bot added the inactive label Nov 14, 2017
@stale
Copy link

stale bot commented Nov 29, 2017

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants