Skip to content

Post process flow default props #231

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

Closed
wants to merge 13 commits into from
Closed

Post process flow default props #231

wants to merge 13 commits into from

Conversation

rosskevin
Copy link
Contributor

Fixes #221

Based on the work by @adityavohra7 in #227 (on master), I have applied a similar change to v3-dev with post processing of defaultProps. Props are not required when they are specified in defaultProps.

Further, I added component_11.js as a diverse test case taken from material-ui. It Is an all flow test that exercises required, optional, and parameterized types.

Use case: mui/material-ui#9222

@rosskevin
Copy link
Contributor Author

Hold, I have a bug to work out.

@rosskevin
Copy link
Contributor Author

rosskevin commented Nov 20, 2017

Ok, this is good to go and passes with the material-ui codebase. I have added every case that broke to component_11.

rosskevin added a commit to alienfast/material-ui that referenced this pull request Nov 20, 2017
rosskevin added a commit to alienfast/material-ui that referenced this pull request Nov 20, 2017
rosskevin added a commit to mui/material-ui that referenced this pull request Nov 20, 2017
* [flow] remove `type DefaultProps`

* [docgen] Remove any flow type casts of default values

* flow type casting now built into react-docgen

reactjs/react-docgen#231

* ignore docs on theme prop

* fixed changed/missing prop docs

* unrecognized doc-gen flow type

* changes/fixes to doc-gen

* re-gen api docs with PR reactjs/react-docgen#231

* Use temp @rosskevin/react-docgen

* re-gen docs - optional props should now be correct
horizontal: 'left',
}: Origin),
labelRowsPerPage: ('Rows per page:': Node),
component: ('div': ElementType),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: What is this doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting to ElementType to avoid other flow errors and avoid typing all default props. Just one alternative. BTW We have since removed flow from our codebase.

Copy link
Collaborator

@danez danez Jan 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for explaining.

Yeah in my company we also dropped flow, because it was too hard to keep up and we had to add lots of workarounds and rewrite code just to make flow happy

@danez
Copy link
Collaborator

danez commented Jan 5, 2018

I posted already in #227, seems this fixes some printing for casts too though

@danez danez closed this Jun 26, 2018
@danez danez changed the base branch from v3-dev to master June 26, 2018 14:10
@danez danez reopened this Jun 26, 2018
@danez
Copy link
Collaborator

danez commented Jul 14, 2018

#227 has been merged and I picked the other changes here: a893dec

@danez danez closed this Jul 14, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants