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
MBS-13558 (I): Migrate React components to Flow component syntax #3225
MBS-13558 (I): Migrate React components to Flow component syntax #3225
Conversation
ccd32fb
to
d91215b
Compare
d91215b
to
f25d67d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skimmed the commits and didn't see any obvious problems. Also did not notice any problems on the subset of pages I checked locally. Since the type checking for component
is supposed to be more strict than the previous function
forms in some ways, I personally think it's fine to merge this given the passing tests.
Should we have a refactoring ticket for this?
This is only used in one place, so there's no need to have a separate types file just for it.
This is only used in one place, so there's no need to have a separate types file just for it.
This is only used in one place, so there's no need to have it in a separate types file.
This is only used in one place, so there's no need to have it in a separate types file.
This is only used in one place, so there's no need to have it in a separate types file.
This is only used in one place, so there's no need to have it in a separate types file.
This is only used in one place, so there's no need to have it in a separate types file.
Map will have the index anyway, so we don't need to pass it in these cases.
In order to make use of the benefits of the syntax, converted components/TagEntiesList here too so I can use its props to avoid defining things twice.
Flow wasn't happy with us passing variables we don't use as part of props. I made some changes on how we do typing here that actually allowed me to drop the separate types file entirely.
No changes to text content, all changes are moving words around for max-len purposes
f25d67d
to
7830caf
Compare
Flow now supports a specific React Component Syntax which reduces the amount of types we need to define manually, and is supposed to be better for type checking as well. I'm converting our React components to this syntax bit by bit with this PR.
This changes all stuff in
root
subfolders except for the largest ones which probably need a PR for each to be less insane:/components
,/edit
,/layout
,/report
and/static
.Note: To be reviewed without whitespace changes (since this adds explicit
return()
calls that require spacing things one more tab in many cases).