Strengthening Flow Types for Core Components #22100
Labels
Bug
Flow
Good first issue
Interested in collaborating? Take a stab at fixing one of these issues.
Help Wanted 
Issues ideal for external contributors.
JavaScript
p: Facebook
Partner: Facebook
Partner
Ran Commands
One of our bots successfully processed a command.
Resolution: Locked
This issue was locked by the bot.
Comments
15 tasks
react-native-bot
added
Ran Commands
|
I'll take |
|
I'll start with |
elicwhite
added
Good first issue
|
Silly bot. |
|
I'll take |
|
I'll take |
3 tasks
|
If anyone needs help with figuring out the callback/return types of native components feel free to ask here, and we can help out! 👍 |
3 tasks
facebook-github-bot
pushed a commit
that referenced
this issue
Nov 4, 2018
Summary: Related to #22100 Turn Flow strict mode on for ScrollViewMock. This file used to declare jest var as `any` but jest module is already typed in root flow folder. Note: I had to use a quick fix for polyfillPromise. See here #22101 - All flow tests succeed. [GENERAL] [ENHANCEMENT] [ScrollViewMock.js] - Flow strict mode Pull Request resolved: #22103 Differential Revision: D12918380 Pulled By: TheSavior fbshipit-source-id: cd3aba47b1a43e76a7da09e15cc2d9cfcdf7f56d
|
I'll take |
This was referenced Nov 4, 2018
|
@empyrical How would you type |
|
Don’t worry about requireNativeComponent. We have another project internally working to remove that function and instead provide an accurate type. |
|
@TheSavior I don't see |
|
Whoops. Looks like another PR from @empyrical landed after I made that list which solved that line in Checkbox. Sorry for the churn. I'll mark that off the list. |
|
I'll take |
3 tasks
facebook-github-bot
pushed a commit
that referenced
this issue
Jan 29, 2019
) Summary: Related to #22100 Enhance last ViewPropTypes flow types. - [x] yarn run prettier - [x] yarn run flow-check-ios - [x] yarn run flow-check-android [GENERAL] [ENHANCEMENT] [ViewPropTypes.js] - Enhance Flow types definitions Pull Request resolved: #23192 Differential Revision: D13858907 Pulled By: cpojer fbshipit-source-id: 3633eb019eca2076bb68393b09d06555876f2c48
|
And we are done with the last PR by @danibonilha. Thanks everyone for helping out with this effort to make our flow types stricter! |
matt-oakes
pushed a commit
to matt-oakes/react-native
that referenced
this issue
Feb 7, 2019
Summary: Related to facebook#22100 Enhance TimePickerAndroid flow types. Turn Flow strict mode on for Libraries/Components/TimePickerAndroid/TimePickerAndroid.android.ios.js. All flow tests succeed. [General] [Changed] - Enhance Flow types definitions Pull Request resolved: facebook#22714 Differential Revision: D13817142 Pulled By: cpojer fbshipit-source-id: 9d0f0b0629966a60d77b73ba8a6bba4e1a4e2337
matt-oakes
pushed a commit
to matt-oakes/react-native
that referenced
this issue
Feb 7, 2019
…ebook#23192) Summary: Related to facebook#22100 Enhance last ViewPropTypes flow types. - [x] yarn run prettier - [x] yarn run flow-check-ios - [x] yarn run flow-check-android [GENERAL] [ENHANCEMENT] [ViewPropTypes.js] - Enhance Flow types definitions Pull Request resolved: facebook#23192 Differential Revision: D13858907 Pulled By: cpojer fbshipit-source-id: 3633eb019eca2076bb68393b09d06555876f2c48
facebook-github-bot
pushed a commit
that referenced
this issue
Mar 8, 2019
Summary: Relates to #22100. I left 2 `$FlowFixMe`s as I was not sure how to handle generic `React.Element<>` and which native props can I pass to ScrollView (would be cool to document it once we got proper types there). I also got rid of `InternalScrollViewType` because we have better typings in original `ScrollView` now. Pull Request resolved: #22301 Reviewed By: TheSavior Differential Revision: D13103990 Pulled By: RSNara fbshipit-source-id: 9664ee9d7f570b00992215e10901e5317f24fe5c
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn Flow strict mode on for ScrollViewMock. This file used to declare jest var as `any` but jest module is already typed in root flow folder. Note: I had to use a quick fix for polyfillPromise. See here facebook#22101 - All flow tests succeed. [GENERAL] [ENHANCEMENT] [ScrollViewMock.js] - Flow strict mode Pull Request resolved: facebook#22103 Differential Revision: D12918380 Pulled By: TheSavior fbshipit-source-id: cd3aba47b1a43e76a7da09e15cc2d9cfcdf7f56d
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn Flow strict mode on for DatePickerIOS. - [x] npm run prettier - [x] npm run flow-check-ios - [x] npm run flow-check-android This error was happend facebook#22101 facebook#22048 [GENERAL] [ENHANCEMENT] [Components/DatePicker/DatePickerIOS.ios.js] - Flow strict mode Pull Request resolved: facebook#22105 Differential Revision: D12920939 Pulled By: TheSavior fbshipit-source-id: aae5ca04d656abb1cf34168e12e44dd50f0a603c
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn Flow strict mode on for KeyBoard - [x] npm run prettier - [ ] npm run flow-check-ios - [ ] npm run flow-check-android This error was happend facebook#22101 facebook#22048 [GENERAL] [ENHANCEMENT] [Components/Keyboard/Keyboard.js] - Flow strict mode Pull Request resolved: facebook#22114 Differential Revision: D12920947 Pulled By: TheSavior fbshipit-source-id: 8d72019efd4d30032ce4784764e5deb9c60e7b01
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Enhance Flow types for RefreshControl specifying `onRefresh` props type. There are still 2 `any` left using `requireNativeComponent` that need to be addressed to turn Flow to strict mode. I went through `RCTRefreshControl` and `AndroidSwipeRefreshLayout` classes to understand where this method came from. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [RefreshControl.js] - Flow onRefresh type Pull Request resolved: facebook#22119 Differential Revision: D12919764 Pulled By: TheSavior fbshipit-source-id: 9ba675be8dbce77d77972acb904fc13c68524831
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn on Flow strict mode for StaticContainer.react This component needed proper Props type definition. I went through the only component (`TabBarItemIOS.ios`) using this to try to know the most appropriate props. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [StaticContainer.react.js] - Flow strict mode Pull Request resolved: facebook#22121 Differential Revision: D12929646 Pulled By: TheSavior fbshipit-source-id: 8826aa7bc83c854efdd71cdb4fba3d7ca98f2fce
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Issue in focus: facebook#22100 The only occurrence of `Object` was replaced with the appropriate flow type A Lint error was encountered in `deepFreezeAndThrowOnMutationInDev-test.js` when running `npm run lint` and was fixed by running `yarn prettier` Pull Request resolved: facebook#22152 Differential Revision: D12930872 Pulled By: RSNara fbshipit-source-id: f9706ed2e49d9ccedfa331594c886d2d3b615db5
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn Flow strict mode on for Libraries/Components/ViewPager/ViewPagerAndroid.android.js - [x] npm run prettier - [x] npm run flow-check-ios - [x] npm run flow-check-android [GENERAL] [ENHANCEMENT] [Libraries/Components/ViewPager/ViewPagerAndroid.android.js] - Flow strict mode Pull Request resolved: facebook#22134 Differential Revision: D12930719 Pulled By: TheSavior fbshipit-source-id: 9519f31b7af27f0497e42fd51f18c19be3692823
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn on Flow strict mode for Slider. Enhanced event type and props callbacks type defs for Slider. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [Slider.js] - Flow strict mode Pull Request resolved: facebook#22127 Differential Revision: D12946817 Pulled By: TheSavior fbshipit-source-id: 631391f70c04fddf0bfa6fec92f5cb769a555547
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Enhance Flow types for TouchableOpacity specifying Touchable event types and TvParallaxPropertiesType. I had to export TvParallaxPropertiesType from TVViewPropTypes file. There are still 1 any left using requireNativeComponent and a dependency to `Touchable` that need to be addressed to turn Flow to strict mode. I guess `Touchable` is a lot more work since there's no flow annotation and it's still good old Mixin. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [TouchableOpacity.js] - Flow types [GENERAL] [ENHANCEMENT] [TVViewPropTypes.js] - Export type Pull Request resolved: facebook#22146 Reviewed By: TheSavior Differential Revision: D12927044 Pulled By: RSNara fbshipit-source-id: c63d805699dd58e2fbc4fd1df4ee0c9f87e2336a
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Enhance TouchableBounce with press event types, callback and hitslop types. There is still some work to do in order to turn flow to strict mode. (requireNativeComponent and render function) - All flow tests succeed. [GENERAL] [ENHANCEMENT] [TouchableBounce.js] - Flow types Pull Request resolved: facebook#22197 Reviewed By: TheSavior Differential Revision: D13032452 Pulled By: RSNara fbshipit-source-id: b21140722ce924698aa15323602e2e3fc663dbb6
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn on Flow strict mode for TextProps. I used ResponseHandlers type definition defined in Text.js. I wanted to move ResponseHandlers type to TextProps and reuse it inside the file. I know I could use $Shape<> to maybe keys but how do I elegantly maybe every values ? Unless having a straightforward solution, I found it clearer to copy paste these types. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [TextProps.js] - Flow strict mode Pull Request resolved: facebook#22122 Reviewed By: TheSavior Differential Revision: D13055759 Pulled By: RSNara fbshipit-source-id: 230b43c7c94d7f82f5727ad11541b0cb98bc5e3a
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Enhance TextInput with callback event types. This is a first draft and I will need more help on this one. Flow checks are successful now but I am not sure types are accurate though. Moreover I find my separation approach kind of dirty for callback event types. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [TextInput.js] - Flow types [GENERAL] [ENHANCEMENT] [TextInputExample.android.js] - Fixing Flow types [GENERAL] [ENHANCEMENT] [TextInputExample.ios.js] - Fixing Flow types [GENERAL] [ENHANCEMENT] [XHRExampleFetch.js] - Fixing Flow types Pull Request resolved: facebook#22250 Reviewed By: TheSavior Differential Revision: D13104820 Pulled By: RSNara fbshipit-source-id: 3fbb98d0ec2b62be676f71ae1053933d9c78485e
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Enhance Flow types for TouchableOpacity specifying underlay functions and TvParallaxPropertiesType. I had to export and enhance TvParallaxPropertiesType from TVViewPropTypes file. This does not break this other PR also using this exported type. facebook#22146 There is still some work to do in order to turn flow to strict mode. - All flow tests succeed. [GENERAL] [ENHANCEMENT] [TouchableHighlight.js] - Flow types [GENERAL] [ENHANCEMENT] [TVViewPropTypes.js] - Export and enhance type Pull Request resolved: facebook#22173 Differential Revision: D13033441 Pulled By: RSNara fbshipit-source-id: 26a38970923dc7e6c02c03da5d08483a3f1fbd36
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Enhance Flow types for Libraries/Components/ScrollResponder.js Pull Request resolved: facebook#22181 Reviewed By: TheSavior Differential Revision: D13032699 Pulled By: RSNara fbshipit-source-id: ca0ce178a96008a71016d033ee1154ad807d6599
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 . I had this issue before(facebook#22154 & facebook#22172). Turn Flow strict mode on for Libraries/Components/TimePickerAndroid/TimePickerAndroid.android.js. - [x] npm run prettier - [x] npm run flow - [x] npm run flow-check-ios - [x] npm run flow-check-android - [x] npm run lint - [x] npm run test - [x] ./scripts/run-android-local-unit-tests.sh [GENERAL][ENHANCEMENT][TimePickerAndroid.android.js] - apply flow strict-local Pull Request resolved: facebook#22188 Reviewed By: TheSavior Differential Revision: D12972356 Pulled By: RSNara fbshipit-source-id: 838604a791dfdc86bacf8b49f6c399920a3f57bc
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 `Libraries/Components/StatusBar/StatusBar.js` Enhance StatusBar with mergePropsStack and _defaultProps. - [x] npm run prettier - [x] npm run flow - [x] npm run flow-check-ios - [x] npm run flow-check-android - [x] npm run lint - [x] npm run test - [x] ./scripts/run-android-local-unit-tests.sh [GENERAL][ENHANCEMENT][StatusBar.js] - apply flow strict-local Pull Request resolved: facebook#22282 Reviewed By: TheSavior Differential Revision: D13103971 Pulled By: RSNara fbshipit-source-id: 27f69c6df3a8a7792fcd595c0ff362943ccab8ca
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Related to facebook#22100 Turn Flow strict mode on for Libraries/Components/Touchable/TouchableNativeFeedback.android.js. Pull Request resolved: facebook#22176 Reviewed By: TheSavior Differential Revision: D13033239 Pulled By: RSNara fbshipit-source-id: 57e89ce16040e6238e01e0437327db943a5f2581
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Bug
Flow
Good first issue
Interested in collaborating? Take a stab at fixing one of these issues.
Help Wanted
Issues ideal for external contributors.
JavaScript
p: Facebook
Partner: Facebook
Partner
Ran Commands
One of our bots successfully processed a command.
Resolution: Locked
This issue was locked by the bot.
We want to tighten the Flow types of the props for our core components. We need the help of the community to comb through these files and improve these types.
Right now a lot of props are typed using
any,Function, orObject.Many of these types are used to validate what we send to native. If JS defines a function that expects a string but Native calls it with a number, applications can crash.
We'd like to fix that by removing all references to
any,Function, orObject. While we ideally want to remove all references of these files from the codebase, there are a lot, so we should prioritize the ones that are used in the props type definitions for components. 😄If your are able to remove all of these weak types from the file, try to change
@flowat the top of the file to@flow strict-local. That should ensure that weak types can't come back to these files in the future.Step 1, Step 2, and Step 3 helped prepare our components for this step.
How to submit quality PRs
Since many of these weak types are used at the boundary between JS and native, you will likely need to read through the native code for these components to see how these props are being used. Unfortunately, Flow will likely pass with invalid types. Paying close attention to what iOS and Android expects will be helpful to ensure the types are accurate.
I also urge those that submit PRs for this to help out with reviewing the PRs for this issue from other contributors. Code review is a great opportunity to learn and improve your own code as well as make sure everyone is on the same page and consistent. If you find tips that would have helped you investigate and improve the types, commenting on this issue with those tips would be appreciated. Help each other. ❤️
Also note that since you are improving these types you will likely help catch a bunch of bugs at Facebook (and elsewhere) where code isn't handling the types correctly. This means that PRs will likely take longer to land then the other issues like this we have asked for help on. This is a good thing, it is direct impact on the stability of React Native projects and catching bugs.
The files
The following is a list of files that I'd like to address first. If you want to take one of these files please comment on this issue with the file name so that others don't work on it as well and waste work. There are plenty of files to go around. 😄
Also,
TextProps.jsandViewPropTypes.jsare probably the files with the most changes necessary. I don't really expect those to be done by one person. Feel free to type a few and send a PR. Once that PR is landed someone else can take it on and type a few more.All the warnings for these files:
The text was updated successfully, but these errors were encountered: