Skip to content

Enum strings in propTypes are wrapped in a tick #57

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

Open
nikgraf opened this issue Feb 16, 2016 · 3 comments
Open

Enum strings in propTypes are wrapped in a tick #57

nikgraf opened this issue Feb 16, 2016 · 3 comments

Comments

@nikgraf
Copy link

nikgraf commented Feb 16, 2016

I defined a propType like this:

type: PropTypes.oneOf(['cute', 'aggressive', 'shy']),

This resulted in:

{
  computed: false,
  value: "'shy'",
}

screen shot 2016-02-16 at 14 17 35

You can see the shy is wrapped in a single tick and in double quotes. Kind of a string in a string ;)]

Is this a feature or bug? I'm happy to submit a PR in case this is a bug.

Additional info: when I use numbers they are also presented as a String:

noseLength: PropTypes.oneOf([33, 42, 88]),
@fkling
Copy link
Member

fkling commented Feb 17, 2016

Mmh, good question. I think the way it is now is how I originally intended it to be (because I imagined the values would be rendered somewhere and as such you want to represent a string with quotes).

Now however I think there are better ways to handle this. I could imagine that value really holds the corresponding JavaScript value (like you expected it), if the value can be resolved) and raw would contain the source (i.e. "'shy'").

This would be a breaking change though. I'd be happy to have a discussion around revisiting the data structure in general and around propTypes in particular.

@nikgraf
Copy link
Author

nikgraf commented Feb 19, 2016

Sound pretty good. I wonder how valuable a big time investment into propTypes is as the React core team announced to move more into the Flow direction (which I'm personally happy about). see facebook/react#1833 (comment)

@fkling totally makes sense regarding the current wrapping of value. In our case we right now try to come up with a normalized structure that suites Flow & PropTypes. That might result in some interesting insight, but right now it's still an exploration phase. see https://github.com/pure-ui/styleguide/blob/master/plugin/normalize-meta-info.js

btw will you be at the React Conf? I'm happy to meet & discuss it in person as well

@fkling
Copy link
Member

fkling commented Feb 23, 2016

@nikgraf: Unfortunately I'm not adding the React Conf. But I hope we find other ways to discuss :)

I wonder how valuable a big time investment into propTypes is as the React core team announced to move more into the Flow direction

I still think code using propTypes will be there for a while, at least at Facebook. I agree though that we don't need to come up with the perfect solution. Because react-docgen evolved over time (we used an early version of this internally for a while), I think I first want to get a good overview again about how all possible types are encoded and what information / fields are available (which is basically #55 ).

Then we should deal with inconsistencies and surprises.

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

2 participants