React-native: Strengthening Flow Types for Core Components

Created on 4 Nov 2018  路  47Comments  路  Source: facebook/react-native

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, or Object.

These types can cause flow to silently skip over portions of your code, which would have otherwise caused type errors.
- From eslint-plugin-flowtype

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, or Object. 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 @flow at 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.js and ViewPropTypes.js are 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.

  • [x] Libraries/Text/TextProps.js
  • [x] Libraries/Components/CheckBox/CheckBox.android.js
  • [x] Libraries/Components/DatePicker/DatePickerIOS.ios.js
  • [x] Libraries/Components/DatePickerAndroid/DatePickerAndroid.android.js
  • [x] Libraries/Components/DatePickerAndroid/DatePickerAndroid.ios.js
  • [x] Libraries/Components/DrawerAndroid/DrawerLayoutAndroid.android.js
  • [x] Libraries/Components/Keyboard/Keyboard.js
  • [x] Libraries/Components/Picker/Picker.js
  • [x] Libraries/Components/Picker/PickerAndroid.android.js
  • [x] Libraries/Components/Picker/PickerIOS.ios.js
  • [x] Libraries/Components/RefreshControl/RefreshControl.js
  • [x] Libraries/Components/ScrollResponder.js
  • [x] Libraries/Components/ScrollView/__mocks__/ScrollViewMock.js
  • [x] Libraries/Components/ScrollView/ScrollView.js
  • [x] Libraries/Components/SegmentedControlIOS/SegmentedControlIOS.ios.js
  • [x] Libraries/Components/Slider/Slider.js
  • [x] Libraries/Components/StaticContainer.react.js
  • [x] Libraries/Components/StatusBar/StatusBar.js
  • [x] Libraries/Components/TextInput/TextInput.js
  • [x] Libraries/Components/TimePickerAndroid/TimePickerAndroid.android.js
  • [x] Libraries/Components/TimePickerAndroid/TimePickerAndroid.ios.js
  • [x] Libraries/Components/Touchable/TouchableBounce.js
  • [x] Libraries/Components/Touchable/TouchableHighlight.js
  • [x] Libraries/Components/Touchable/TouchableNativeFeedback.android.js
  • [x] Libraries/Components/Touchable/TouchableOpacity.js
  • [x] Libraries/Components/Touchable/TouchableWithoutFeedback.js
  • [x] Libraries/Components/View/ViewPropTypes.js
  • [x] Libraries/Components/ViewPager/ViewPagerAndroid.android.js



All the warnings for these files:

Libraries/Text/TextProps.js
    109:23  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
    110:22  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
    111:25  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
    112:27  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
    113:36  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
    114:32  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/CheckBox/CheckBox.android.js
  121:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/DatePicker/DatePickerIOS.ios.js
  27:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/DatePickerAndroid/DatePickerAndroid.android.js
  18:29  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  68:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  68:47  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/DatePickerAndroid/DatePickerAndroid.ios.js
  14:23  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  14:40  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/DrawerAndroid/DrawerLayoutAndroid.android.js
  155:32  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Keyboard/Keyboard.js
  124:58  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Picker/Picker.js
  35:11  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  66:19  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  73:31  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Picker/PickerAndroid.android.js
  37:19  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  40:31  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Picker/PickerIOS.ios.js
  30:15  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  37:11  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  65:31  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  66:18  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  76:12  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/RefreshControl/RefreshControl.js
  90:16  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/ScrollResponder.js
  116:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  412:49  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    
  537:17  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    

Libraries/Components/ScrollView/__mocks__/ScrollViewMock.js
  15:19  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/ScrollView/InternalScrollViewType.js
  29:25  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  30:24  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  31:23  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  34:17  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/ScrollView/ScrollView.js
  224:20  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  407:28  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  411:26  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  417:15  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  421:24  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  426:22  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  436:26  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  595:35  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  609:34  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort      
  613:33  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort      
  738:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  765:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  774:39  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   

Libraries/Components/SegmentedControlIOS/SegmentedControlIOS.ios.js
  36:20  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  37:15  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  45:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   

Libraries/Components/Slider/Slider.js
   27:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  121:20  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  128:24  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/StaticContainer.react.js
  30:47  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  31:36  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/StatusBar/StatusBar.js
  110:21  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  111:18  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  112:4   warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  127:34  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    
  127:40  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    

Libraries/Components/TextInput/TextInput.js
   58:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  187:13  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  188:14  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  189:15  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  190:19  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  191:26  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  192:18  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  193:19  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  194:24  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  195:22  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  196:17  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  197:15  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  795:22  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  900:32  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort      

Libraries/Components/TimePickerAndroid/TimePickerAndroid.android.js
  55:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  55:47  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/TimePickerAndroid/TimePickerAndroid.ios.js
  14:23  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  14:40  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Touchable/TouchableBounce.js
   27:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
   39:28  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   40:31  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   98:17  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  150:37  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   

Libraries/Components/Touchable/TouchableHighlight.js
  42:27  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  52:21  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  53:21  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Touchable/TouchableNativeFeedback.android.js
  41:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Touchable/TouchableOpacity.js
  28:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  34:27  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/Touchable/TouchableWithoutFeedback.js
   45:32  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort      
   47:13  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort      
   48:7   warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort      
   61:13  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   62:14  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   63:15  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   64:18  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   65:14  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   66:16  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   67:17  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  180:57  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  209:37  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   

Libraries/Components/View/ViewPropTypes.js
   35:28  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   43:25  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   64:17  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   68:20  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   69:27  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   70:17  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   71:24  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   72:18  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   73:25  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   74:19  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   75:26  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
   93:31  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  104:38  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  115:23  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  125:22  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  136:24  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  146:25  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  148:23  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  149:21  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  162:27  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  173:36  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  183:32  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  194:39  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  198:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  199:30  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   

Libraries/Components/ViewPager/ViewPagerAndroid.android.js
  26:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort   
  50:19  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  60:31  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  68:21  warning  The `Function` type is too generic and could lead to hard to debug errors. Please use only as a last resort 

Libraries/Components/WebView/WebView.ios.js
   57:11  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    
   58:9   warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    
   59:16  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    
   62:14  warning  The `Object` type is too generic and could lead to hard to debug errors. Please use only as a last resort 
  659:26  warning  The `any` type is too generic and could lead to hard to debug errors. Please use only as a last resort    

Bug Flow Good first issue Help Wanted JavaScript Partner Ran Commands Locked Facebook

Most helpful comment

I'll take Libraries/Components/WebView/WebView.ios.js

All 47 comments

I'll take Libraries/Components/DatePicker/DatePickerIOS.ios.js

I'll start with Libraries/Components/ScrollView/__mocks__/ScrollViewMock.js

Silly bot.

I'll take Libraries/Components/RefreshControl/RefreshControl.js

I'll take Libraries/Components/DatePicker/DatePickerAndroid.ios.js , Libraries/Components/DatePickerAndroid/DatePickerAndroid. android.js

If anyone needs help with figuring out the callback/return types of native components feel free to ask here, and we can help out! 馃憤

I'll take Libraries/Components/Keyboard/Keyboard.js, Libraries/Components/Picker/Picker.js

@empyrical How would you type requireNativeComponent ?

Don鈥檛 worry about requireNativeComponent. We have another project internally working to remove that function and instead provide an accurate type.

@TheSavior I don't see Object on line 121 or anywhere else in CheckBox.android.js. What I do see is any on line 81 but it's in reference to requireNativeComponent, which you said above we shouldn't worry about.

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 Libraries/Components/StaticContainer.react.js

I'll take Libraries/Text/TextProps.js

I could take Libraries/Components/Slider/Slider.js as well

I'll take Libraries/Components/Picker/PickerAndroid.android.js, Libraries/Components/Picker/PickerIOS.ios.js

I'll take Libraries/Components/WebView/WebView.ios.js

I'll take Libraries/Components/StatusBar/StatusBar.js

I'll take Libraries/Components/Touchable/TouchableBounce.js !

I'll take below.

  • Libraries/Components/TimePickerAndroid/TimePickerAndroid.android.js
  • Libraries/Components/TimePickerAndroid/TimePickerAndroid.ios.js.

I'll take Libraries/Components/Touchable/TouchableOpacity.js

I'll take Libraries/Components/View/ViewPropTypes.js

I'll take Libraries/Components/ViewPager/ViewPagerAndroid.android.js

I'll take Libraries/Components/Touchable/TouchableWithoutFeedback.js ~~

I'll take Libraries/Components/Touchable/TouchableHighlight.js

I took Libraries/Components/DrawerAndroid/DrawerLayoutAndroid.android.js

I'll take Libraries/Components/ScrollResponder.js

I'll take Libraries/Components/Touchable/TouchableBounce.js

I'll take Libraries/Components/SegmentedControlIOS/SegmentedControlIOS.ios.js

I'll take Libraries/Components/Touchable/TouchableNativeFeedback.android.js

I think the fix is not needed in Libraries/Components/SegmentedControlIOS/SegmentedControlIOS.ios.js
Maybe #21888 fixed this issue 馃憤

I'll take Libraries/Components/TextInput/TextInput.js

@exced, TextInput has an open PR to convert it to an es6 class by @empyrical. You might have merge conflicts if you change more things in that file before it lands.

Yeah, you might want to wait - when it lands, however, there will still need to be work done on it to make it pass flow strict-local however! 馃憤

@TheSavior @empyrical Alright I'll wait

I'll take Libraries/Components/StatusBar/StatusBar.js
UPDATE: @watanabeyu opened a PR for it. Thanks

@exced The ES6 conversion had to be rolled back, so you should be able to take this on.

I'll take Libraries/Components/StatusBar/StatusBar.js

Happy to take Libraries/Text/TextProps

@thymikee There's this PR opened #22122
Have you seen issues in it ? Thanks

Oh, I haven't noticed this. Will try to find something different then ;)
Picked up ScrollView

I'll take Libraries/Components/DatePickerAndroid/DatePickerAndroid.android.js and Libraries/Components/DatePickerAndroid/DatePickerAndroid.ios.js

Hey, I was wondering, isn't Libraries/Components/View/ViewPropTypes.js done ? 馃槃

I think it is close but there are one or two callsites that need strengthening in that file still.

Thanks @TheSavior

We still have one file to go here but I have created another issue with the next step of this project. I'd love any help we can get. You all have been amazing. 鉂わ笍

https://github.com/facebook/react-native/issues/22990

Anybody can to explain me why import type {SyntheticEvent} from 'CoreEventTypes'; works?
Why flow understand CoreEventTypes alias ?

Because module.system.haste.paths.whitelist=<PROJECT_ROOT>/Libraries/.* in .flowconfig?
But .flowconfig of react native project containts module.system.haste.paths.whitelist=<PROJECT_ROOT>/node_modules/react-native/Libraries/.* and I can't using alias CoreEventTypes. Why?

And we are done with the last PR by @danibonilha. Thanks everyone for helping out with this effort to make our flow types stricter!

Was this page helpful?
0 / 5 - 0 ratings