React-native: Removing propTypes from core components

[TheSavior]

We have been using Flow for our components for a long while and have enjoyed much stricter type safety, enabling us to validate our code at compile time instead of with propTypes at runtime. We want to replace the remaining callsites of propTypes with Flow and deprecate the propType shapes exposed by React Native core. To do this, we need your help!

This is step 1 of our multi step goal. Step 2 can be found here.

There are two classes of things that need to happen to complete this work. The first step is removing references from propTypes in our core components. The following are the individual files that need to be converted / updated. Each file should be converted in its own PR and each file is about the size of a good first issue to learn and get familiar with a part of the React Native codebase. If you’d like to convert one, pick one from the list, comment on this issue that you are interested in converting it, and follow the tips in @empyrical's comment for help successfully updating that file.

Remove callsites from components:

• [x] Libraries/Components/Button.js -> Delete PropTypes
  
  
  
  • [x] MaskedViewIOS.ios.js -> Delete PropTypes
  
  
  • [x] Picker/Picker.js -> Delete PropTypes
  
  
  • [x] PickerAndroid.android.js -> Delete PropTypes
  
  
  • [x] StaticRenderer.js -> Delete PropTypes
  
  
  • [x] StatusBar/StatusBar.js -> Delete PropTypes
  
  
  • [x] TabBarIOS/TabBarIOS.ios.js -> Delete PropTypes
  
  
  • [x] TabBarIOS/TabBarItemIOS.ios.js -> Convert to Flow Types
  
  
  • [x] ViewPagerAndroid.android.js -> Delete PropTypes
  
  
  • [x] IncrementalPresenter.js -> Delete PropTypes, leave contextTypes
  
  
  • [x] SwipeableFlatList.js -> Delete PropTypes
  
  
  • [x] SwipeableListView.js -> Delete PropTypes
  
  
  • [x] SwipeableQuickActions.js -> DeletePropTypes (style changed to ViewStyleProp)
  
  
  • [x] ElementProperties.js -> Delete PropTypes
  
  
  • [x] InspectorPanel -> Delete PropTypes
  
  
  • [x] InspectorOverlay.js -> Delete PropTypes
  
  
  • [x] SnapshotViewIOS.ios.js -> Delete PropTypes
  
  
  • [x] LinkingExample.js -> Convert to Flow Types
  
  
  • [x] RNTesterBlock.js -> Delete PropTypes, strengthen Flow Types
  
  
  • [x] RNTesterButton.js -> Delete PropTypes
  
  
  • [x] RNTesterPage.js -> Delete PropTypes
  
  
  • [x] Modal.js -> Convert to Flow Types, slightly bigger task
  
  
  • [x] IntegrationTestHarnessTest
  
  
  • [x] InputAccessoryView -> no propTypes, but flow types need to get cleaned up
  
  

Move and Rename custom propType definitions

For these, I think we should create a new folder, react-native/Libraries/DeprecatedPropTypes. Many of these files have both the Flow definition and the propTypes definition. "Split" means split these into two files, in the example of EdgeInsetsPropType it would mean having an EdgeInsetsPropType.js file that contains the commented flow types, and a DeprecatedEdgeInsetsPropType.js file inside the DeprecatedPropTypes folder. Eventually that folder will become the source for the new repo that we move out.

• [x] Create a new folder at react-native/Libraries/DeprecatedPropTypes (should be done as part of one of the other tasks)
• [x] DeprecatedViewPropTypes -> Move, comments to ViewPropTypes
• [x] EdgeInsetsPropType -> Split
• [x] PlatformViewPropTypes -> Move
• [x] TVViewPropTypes -> Split
• [x] StyleSheetPropType -> Move
• [x] createStrictShapeTypeChecker -> Move
• [x] ViewAccessibility -> Split (in progress)
• [x] ViewStylePropTypes -> Move
• [x] ImageProps -> Split (in progress)
• [x] ImageSourcePropType -> Move
• [x] ImageStylePropTypes -> Move
• [x] TextPropTypes -> Move
• [x] ColorPropType -> Move
• [x] LayoutPropTypes -> Move, comments to StyleSheetTypes
• [x] ShadowPropTypesIOS -> Move, comments to StyleSheetTypes
• [x] TransformPropTypes -> Move, comments to StyleSheetTypes
• [x] PointPropType -> Split

[empyrical]

Tips for removing propTypes from components

To best demonstrate what needs to be done, I'll walk through a component that I converted over (Modal)

Here's before it was converted: https://github.com/facebook/react-native/blob/1151c096dab17e5d9a6ac05b61aacecd4305f3db/Libraries/Modal/Modal.js

And after:

https://github.com/facebook/react-native/blob/master/Libraries/Modal/Modal.js

Before, you can see that it had no flow types for its props at all:

class Modal extends React.Component<Object> {

Its props type was just Object, so it would accept any props as valid. Not very precise!

So we will need to create a flow type to work with:

type Props = $ReadOnly<{|
 //...
|}>;

The pipe operator in the brackets mean this type is exact, and the $ReadOnly utility type helps ensure that the values of the props are not manipulated inside of the components.

Then add it to the component like so:

class Modal extends React.Component<Props> {

If the component has state, the syntax is like so:

class Foo extends React.Component<Props, State> {

Before we get to the propTypes, I'd like to point out this part of Modal:

 static contextTypes = {
   rootTag: PropTypes.number,
 };

 // ...

 static childContextTypes = {
   virtualizedList: PropTypes.object,
 };

These usages of the 'prop-types' module aren't being removed yet - so leave them in for now.

For a proptype like an enum:

   /**
    * The `animationType` prop controls how the modal animates.
    *
    * See https://facebook.github.io/react-native/docs/modal.html#animationtype
    */
   animationType: PropTypes.oneOf(['none', 'slide', 'fade']),

It can be ported to Flow like so:

 /**
  * The `animationType` prop controls how the modal animates.
  *
  * See https://facebook.github.io/react-native/docs/modal.html#animationtype
  */
 animationType?: ?('none' | 'slide' | 'fade'),

The question marks mean the property is nullable. Ideally, they should be made nullable where possible.

A great deal of the proptypes are primitive types that can be ported over easily, like:

• PropTypes.bool -> boolean

• PropTypes.string -> string

• PropTypes.number -> number

For PropTypes.arrayOf, you'll want to use $ReadOnlyArray<> rather than Array<>, like so:

   supportedOrientations: PropTypes.arrayOf(
     PropTypes.oneOf([
       'portrait',
       // ... snipped
     ]),
   ),
 // Would become
 supportedOrientations?: ?$ReadOnlyArray<
    | 'portrait'
    // ... snipped
 >,

PropTypes.func is a callback - ideally you should try and fully type the arguments they take, but some of these aren't currently documented so it can be hard to find what their proper types should be. The vast majority of cases, they are SyntheticEvents and you can import them from CoreEventTypes using this syntax:

import type {SyntheticEvent} from  'CoreEventTypes';

And you'd type a callback like this:

 /**
  * The `onRequestClose` callback is called when the user taps the hardware
  * back button on Android or the menu button on Apple TV.
  *
  * This is required on Apple TV and Android.
  *
  * See https://facebook.github.io/react-native/docs/modal.html#onrequestclose
  */
 onRequestClose?: ?(event?: SyntheticEvent<null>) => mixed,

Use mixed rather than void as the return type for these.

I found that the value of the event is null by looking at the native code where the event is fired, and seeing that the value is just nil

 dispatch_block_t completionBlock = ^{
   if (modalHostView.onShow) {
     modalHostView.onShow(nil);
   }
 };

If you're unable to find what the value should be, stub it out using ?Function and ask for help in your pull request.

When you're done, run this command to make sure your code is properly formatted:

yarn prettier
# Or if you don't have yarn
npm run prettier

And run Flow to make sure the types are okay:

yarn flow-check-ios # for *.ios.js files
yarn flow-check-android # for *.android.js files
# Or if you don't have yarn
npm run flow-check-ios # for *.ios.js files
npm run flow-check-android # for *.android.js files

Thank you for helping out, and if you have any questions feel free to ask!

[richardcann]

Im taking on the Libraries/Components/Button.js delete PropTypes conversion

[TheSavior]

@rmcp1g15 Thanks for jumping in! It looks like that one is already checked off (completed). Would you be willing to take a different one?

[empyrical]

@rmcp1g15 That was already converted over, though I realize I forgot this import in it:

const ColorPropType = require('ColorPropType');

[richardcann]

@TheSavior Ok! yeah ill take on StaticRenderer.js and deleting propTypes

[kdastan]

@TheSavior I'll take ElementProperties.js, InspectorOverlay.js

[nd-02110114]

@TheSavior
I'll take MaskedViewIOS.ios.js, ViewPagerAndroid.android.js

[peaonunes]

Just noticed that there is no proptypes in PickerAndroid.android.js, can you confirm?I will take ViewPagerAndroid.android.js instead.

[TheSavior]

PickerAndroid

Looks like PickerAndroid.android.js was taken care of in this commit and I forgot to mark it off. Thanks for flagging!

[danibonilha]

@TheSavior Hi, I'll take the creation of DeprecatedPropTypes' folder and split EdgeInsetsPropType

[empyrical]

@danibonilha Just giving you a heads up about this pull request I made:

https://github.com/facebook/react-native/pull/21349

[danibonilha]

@empyrical okay, thanks, I'll just split EdgeInsetsPropType then.

[exced]

Should ViewStylePropTypes be splitted as well ? (since there's a dependency with PlatformViewPropTypes).

[empyrical]

Yep, that should get split out too

[gvarandas]

I'll take TransformPropTypes for the "move and rename" task.

[danibonilha]

I'll move StyleSheetPropType and rename it to Deprecated

[empyrical]

I am currently focusing more on SwipableFlatList as it is turning out to be a bit complex. If anyone else wants to take on SwipeableQuickActions.js it's up for grabs!

edit: ended up taking it on anyways

[gvarandas]

I'll move ShadowPropTypesIOS as well.

[dkaushik95]

Hey, Can I take InspectorPanel?

[TheSavior]

@dkaushik95, sounds good!

[dkaushik95]

Need help with 5 functions definitions to resolve issues of PR 21392

Function names:

• setInspecting
• setPerfing
• setTouchTargeting
• setNetworking
• onClick

[UPDATE]- done 😄

[gvarandas]

I wanted to thank the whole React team to give this opportunity for people to contribute to a project they use and believe.
Sometimes contributing to a big Open Source project may seem like a distant reality and efforts like this are what motivate developers to overcome that fear or insecurity to submit their first PR.

Props to you, guys. ❤️

[sopranolinist]

I'd like to take the ImageProps split.

[danibonilha]

I'm taking ViewAccessibility =)

[dkaushik95]

Hey. While I was doing the InspectorPanel, I realized that a lot of components are being reused and their types can be exported reused inside Inspector. Should I do another PR for having something like InspectorTypes which has something like InspectorPanelType, InspectorButtonType and BoxInspectorTypes ?

[TheSavior]

Yeah, seems like that duplication is not great and that seems like a good idea!

[yash324]

could I work on createStrictShapeTypeChecker please? Looks like we could split CameraRoll.js too

[TheSavior]

Yeah, that would be great

[TheSavior]

I wanted to thank the whole React team to give this opportunity for people to contribute to a project they use and believe.
Sometimes contributing to a big Open Source project may seem like a distant reality and efforts like this are what motivate developers to overcome that fear or insecurity to submit their first PR.

Props to you, guys. ❤️

@gvarandas I'm glad you have appreciated this and that you felt like this process was welcoming.

To all of you that have contributed to this task so far, thank you so much for your help. I was completely surprised by all of your interest in contributing to the project and support in completing these tasks. @empyrical, @rmcp1g15, @kdastan, @nd-02110114, @peaonunes, @danibonilha, @exced, @ronal2do, @dkaushik95, @gvarandas, @sopranolinist, and @Thomas0c, this work and your excitement has impacted us here at Facebook. We see that the community wants these opportunities, so we're thinking about how we can do more of this in the future.

This work was a major first step to one of our team's goals of having our components strictly typed and modernized. We would like to figure out how to continue providing opportunities to all of you as well as others to contribute to this goal.

The next step is removing mixins from the components that are still using createReactClass. I have created a new issue for removing TimerMixin and if you are interested I would love to get your help on fixing up those as well.

Thanks again for all of your contributions so far.

P.S. To those of you who have contributed to this task, we'd like to send you a token of our appreciation. Please email Hector Ramos at hramos at fb dot com with your Github username, full name, and address. We've got something for you. :)

[yash324]

Hi guys, I tried adding the fixes for createStrictShapeTypeChecker but am unable to run the flow tests on my local machine. I'm getting 1222 'Cannot resolve module' errors on the master branch itself. Spent a lot of time trying to find a way to fix it but in vain. I'd really appreciate some help on this :)

[TheSavior]

@yash324, are you running on Windows? I heard somewhere that Flow for React Native Flow is broken on master for Windows. Unfortunately, nobody around me has a Windows computer to be able to test / fix it. >_<

[yash324]

Duh! I guess you can remove my assignment then :(

[TheSavior]

Can anyone take moving createStrictShapeTypeChecker? That is the only thing left on this issue and then we can close it out! ❤️

[exced]

I can take it @TheSavior

[TheSavior]

This step is all done! Yay!

[douglasjunior]

How to reuse these propTypes now?

In some libraries that I maintain, I do something like this:

import React, { Component } from 'react';
import { Modal } from 'react-native';
import PropTypes from 'prop-types';

class Dialog extends Component {
    ...
} 

Dialog.propTypes = {
   ...Modal.propTypes,
   myCustomProp: PropTypes.string,
}

Any suggestions?

[TheSavior]

We plan to pull these propTypes out into another npm package that the community can maintain but the goal is that we don’t want to support propTypes, we recommend people use Flow or TypeScript instead.