Removing propTypes from core components #21342

TheSavior · 2018-09-26T23:48:51Z

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:

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.

The text was updated successfully, but these errors were encountered:

empyrical · 2018-09-26T23:49:43Z

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 · 2018-09-27T00:15:45Z

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

TheSavior · 2018-09-27T00:19:33Z

@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 · 2018-09-27T00:19:53Z

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

const ColorPropType = require('ColorPropType');

richardcann · 2018-09-27T00:23:39Z

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

kdastan · 2018-09-27T00:47:24Z

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

Related to facebook#21342

nissy-dev · 2018-09-27T01:28:41Z

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

peaonunes · 2018-09-27T01:31:12Z

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

TheSavior · 2018-09-27T01:37:06Z

PickerAndroid

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

danibonilha · 2018-09-27T02:22:19Z

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

empyrical · 2018-09-27T03:00:17Z

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

#21349

danibonilha · 2018-09-27T03:04:25Z

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

exced · 2018-09-27T03:52:35Z

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

empyrical · 2018-09-27T04:26:24Z

Yep, that should get split out too

Summary: This PR split PointPropType.js into PointPropType.js with Flow definition and Libraries/DeprecatedPointPropType.js remaining with PropTypes definition. Related to #21342 Pull Request resolved: #21355 Differential Revision: D10081399 Pulled By: TheSavior fbshipit-source-id: 2283ff3fbda6b0f525742336f92fd6279250b874

douglasjunior · 2019-01-28T18:12:55Z

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?

…1342

TheSavior · 2019-01-28T18:41:45Z

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.

Summary: This PR split PointPropType.js into PointPropType.js with Flow definition and Libraries/DeprecatedPointPropType.js remaining with PropTypes definition. Related to facebook#21342 Pull Request resolved: facebook#21355 Differential Revision: D10081399 Pulled By: TheSavior fbshipit-source-id: 2283ff3fbda6b0f525742336f92fd6279250b874

Summary: Related to facebook#21342 Pull Request resolved: facebook#21350 Differential Revision: D10081454 Pulled By: TheSavior fbshipit-source-id: db27a1f23c643b7d6d73136254eff91625419583

Summary: related facebook#21342 This is a first PR for this repo. So, if there are any problem, please tell me 🙇 TODO * delete props types * apply read only interface CheckList - [x] `yarn prettier` - [x] `yarn flow-check-ios` Pull Request resolved: facebook#21346 Differential Revision: D10081962 Pulled By: TheSavior fbshipit-source-id: 32387c58f180b9aa5f854e323a4bb29aa73f04c8

Summary: Related to facebook#21342 Pull Request resolved: facebook#21345 Differential Revision: D10081976 Pulled By: TheSavior fbshipit-source-id: d6a905704fc5c2f10a6a8552f04e9c3feaeb147b

Summary: Related to facebook#21342 Pull Request resolved: facebook#21343 Reviewed By: RSNara Differential Revision: D10080219 Pulled By: TheSavior fbshipit-source-id: 3a9108208fe6aaa7a30b99f24ceef03e884ec48a

Summary: This PR splits EdgeInsetsPropTypes into EdgeInsetsPropTypes with only flow types and DeprecatedEdgeInsetsPropTypes inside DeprecatedProptypes with only PropTypes. Related to facebook#21342 Pull Request resolved: facebook#21351 Reviewed By: RSNara Differential Revision: D10081512 Pulled By: TheSavior fbshipit-source-id: 267a6fbb455e02dd7f2b0f3b59790e96387eaa09

Summary: related facebook#21342 The `render` function, I was not able to specifically type since the props passed to it may vary. At the moment only `renderRow` function from ListView component is using StaticRenderer, and the type of the renderRow function is `Function`. Let me know what your thoughts are on this. Thank you Pull Request resolved: facebook#21348 Differential Revision: D10084990 Pulled By: TheSavior fbshipit-source-id: a87a8d4976c6ffaf83dc0fddc758869dbc2e2803

…21375) Summary: related facebook#21342 move TransformPropTypes.js fix flow error - [x] yarn prettier - [x] yarn flow-check-android - [x] yarn flow-check-ios All flow checks pass. [GENERAL] [ENHANCEMENT] [DeprecatedTransformPropTypes.js] - Created. Pull Request resolved: facebook#21375 Differential Revision: D10095453 Pulled By: TheSavior fbshipit-source-id: fbf677a000e3c6c0bd31e915dcafbd2d561be6e3

…cebook#21372) Summary: Related to facebook#21342 - Split TVViewPropTypes - PlatformViewPropTypes (dependencies) flow types and old prop-type definitions - ViewStylePropTypes (dependencies) rm prop-type Flow tests succeed [GENERAL] [ENHANCEMENT] [TVViewPropTypes.js] - rm prop-types [GENERAL] [ENHANCEMENT] [PlatformViewPropTypes.android.js] - replace prop-types by Flow [GENERAL] [ENHANCEMENT] [PlatformViewPropTypes.ios.js] - replace prop-types by Flow [GENERAL] [ENHANCEMENT] [DeprecatedTVViewPropTypes.js] - old prop-types Pull Request resolved: facebook#21372 Differential Revision: D10095528 Pulled By: TheSavior fbshipit-source-id: 4fc52ab194f680f95aabefedcbf119d6897672b7

Summary: related facebook#21342 This is a first PR for this repo. So, if there are any problem, please tell me 🙇 TODO * delete props types * apply read only interface CheckList - [x] `yarn prettier` - [x] `yarn flow-check-android` Pull Request resolved: facebook#21347 Differential Revision: D10095503 Pulled By: TheSavior fbshipit-source-id: fd1adb5edf19234ae8ae9f3fe732b03a521eb82b

…ok#21371) Summary: related facebook#21342 TODO * move ColorPropType.js * fix flow error CheckList - [x] `yarn prettier` - [x] `yarn flow-check-android` - [x] `yarn flow-check-ios` All flow checks pass. [GENERAL] [ENHANCEMENT] [DeprecatedColorPropType.js] - Created. Pull Request resolved: facebook#21371 Reviewed By: RSNara Differential Revision: D10087818 Pulled By: TheSavior fbshipit-source-id: 48088b441699886eef1fff3aafc2ca6015455006

…21379) Summary: related facebook#21342 move TransformPropTypes.js fix flow error - [x] yarn prettier - [x] yarn flow-check-android - [x] yarn flow-check-ios All flow checks pass. [GENERAL] [ENHANCEMENT] [DeprecatedShadowPropTypesIOS.js] - Created. Pull Request resolved: facebook#21379 Differential Revision: D10098750 Pulled By: TheSavior fbshipit-source-id: f7f2e4bf7b837c00a14b1fbd930d1b29ffb63549

…acebook#21380) Summary: This PR moves and renames all references of StyleSheetPropType to DeprecatedStyleSheetPropType Related to facebook#21342 Pull Request resolved: facebook#21380 Differential Revision: D10098216 Pulled By: TheSavior fbshipit-source-id: da8d927f87bd37cdabc315e0aa17b6ae208f7124

Summary: Part of: facebook#21342 This PR removes prop types from `SwipeableRow`, and converts it from a `createReactClass` to an ES6 class. Pull Request resolved: facebook#21386 Differential Revision: D10100555 Pulled By: TheSavior fbshipit-source-id: ab350546f4fa6f1ed3fdeae07e342890af6d9a22

…21384) Summary: Part of: facebook#21342 This PR removes the prop types for the components `SwipeableFlatList` and `SwipeableQuickActions`. The props for `SwipeableFlatList` have been left as not $ReadOnly, because it needs the types in `Libraries/Lists/*` to also be cleaned up. A todo notice has been added. Pull Request resolved: facebook#21384 Differential Revision: D10099694 Pulled By: TheSavior fbshipit-source-id: 424b900942c9a7889b664f351f79abee55923430

…facebook#21387) Summary: related facebook#21342 TODO * move ImageSourcePropType.js, ImageStylePropTypes.js, TextPropTypes.js * fix flow error CheckList - [x] `yarn prettier` - [x] `yarn flow-check-android` - [x] `yarn flow-check-ios` All flow checks pass. [GENERAL] [ENHANCEMENT] [DeprecatedImageSourcePropType.js] - Created. [GENERAL] [ENHANCEMENT] [DeprecatedImageStylePropTypes.js] - Created. [GENERAL] [ENHANCEMENT] [DeprecatedTextPropTypes.js] - Created. Pull Request resolved: facebook#21387 Reviewed By: TheSavior Differential Revision: D10099753 Pulled By: RSNara fbshipit-source-id: c907af6d1549ee42de1a2e17f278998e8422110f

Summary: Related to facebook#21342 * Renamed ViewStyleProps to DeprecatedViewStyleProps. * Moved propType declaration to `react-native/Libraries/DeprecatedPropTypes` * ImageProps.js: moved propType declarations to DeprecatedImageProps.js. Pull Request resolved: facebook#21415 Reviewed By: TheSavior Differential Revision: D10119599 Pulled By: RSNara fbshipit-source-id: 67674039a88dcd570973c7062f86ebdbd6987d28

…ook#21370) Summary: related facebook#21342 TODO * move LayoutPropType.js * fix flow error CheckList - [x] `yarn prettier` - [x] `yarn flow-check-android` - [x] `yarn flow-check-ios` All flow checks pass. [GENERAL] [ENHANCEMENT] [DeprecatedLayoutPropTypes.js] - Created. [GENERAL] [ENHANCEMENT] [StyleSheetTypes.js] - add comments. Pull Request resolved: facebook#21370 Differential Revision: D10099715 Pulled By: RSNara fbshipit-source-id: d0515fe0d56d9ed2fde50cc0bfb75b63aded1f5d

…nTestHarnessTest and InputAccessoryView (facebook#21397) Summary: related facebook#21342 Pull Request resolved: facebook#21397 Reviewed By: TheSavior Differential Revision: D10119623 Pulled By: RSNara fbshipit-source-id: 16bdb3d5bf90c24b597bbc12fc416a50a0aa2bb1

Summary: Related to facebook#21342 Pull Request resolved: facebook#21392 Reviewed By: TheSavior Differential Revision: D10129812 Pulled By: RSNara fbshipit-source-id: 79e900b56eb043452ce8e13e998a9ad8d4443897

Summary: Related to facebook#21342 - Split ImageProps.js: moved propType declarations to DeprecatedImageProps.js - Renamed ImageProps references to DeprecatedImageProps in Image.ios.js Pull Request resolved: facebook#21411 Reviewed By: TheSavior Differential Revision: D10146178 Pulled By: RSNara fbshipit-source-id: 4db15eaaa8822e834af347d1927991dff1c427cb

facebook#21422) Summary: This PR splits and renames all references of ViewAccessibility to DeprecatedViewAccessibility Related to facebook#21342 Pull Request resolved: facebook#21422 Reviewed By: yungsters Differential Revision: D10132659 Pulled By: RSNara fbshipit-source-id: 68c371230c69ed37c3e44bf8a36043adb04afc78

Summary: Related to facebook#21342 Move createStrictShapeTypeChecker to DeprecatedCreateStrictShapeTypeChecker. Reviewed By: hramos Differential Revision: D10341526 fbshipit-source-id: 30e7f22ae574af620ead9c1a0766f00611b282b6

TheSavior · 2022-01-21T07:32:57Z

It's been a long journey, but @yungsters just landed 23717c6 which removed the remaining prop-types usages and the DeprecatedPropTypes files that you all helped create from React Native.

We are finally free of prop-types!

TheSavior added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. JavaScript labels Sep 26, 2018

TheSavior changed the title ~~[Needs Help] Removing propTypes from core components~~ [Need Help] Removing propTypes from core components Sep 26, 2018

TheSavior mentioned this issue Sep 26, 2018

Deprecate and eventually remove propTypes from core react-native-community/discussions-and-proposals#29

Closed

hramos added the Help Wanted Issues ideal for external contributors. label Sep 27, 2018

julioxavierr pushed a commit to julioxavierr/react-native that referenced this issue Sep 27, 2018

Remove PropTypes from IncrementalPresenter

b8c7ba5

Related to facebook#21342

julioxavierr mentioned this issue Sep 27, 2018

Remove PropTypes from IncrementalPresenter #21343

Closed

kdastan mentioned this issue Sep 27, 2018

Remove PropTypes from InspectorOverlay #21345

Closed

nissy-dev mentioned this issue Sep 27, 2018

Remove PropTypes from MaskedViewIOS.ios.js #21346

Closed

2 tasks

nissy-dev mentioned this issue Sep 27, 2018

Remove PropTypes from ViewerPagerAndroid.android.js #21347

Closed

2 tasks

richardcann mentioned this issue Sep 27, 2018

StaticRenderer: Removed prop types #21348

Closed

kdastan mentioned this issue Sep 27, 2018

Remove PropTypes from ElementProperties #21350

Closed

danibonilha mentioned this issue Sep 27, 2018

Split EdgeInsets in DeprecatedPropTypes and flow types #21351

Closed

exced mentioned this issue Sep 27, 2018

[PropTypes -> Flow] TVViewPropTypes, PlatformViewPropTypes, ViewStylePropTypes #21352

Closed

peaonunes mentioned this issue Sep 27, 2018

[PropTypes -> Flow] Split PointPropType #21355

Closed

douglasjunior added a commit to douglasjunior/react-native-simple-dialogs that referenced this issue Jan 28, 2019

Fixing propTypes to works with RN 0.58 due to facebook/react-native#2…

f5ab53c

…1342

martin-nebel mentioned this issue Feb 7, 2019

Modal.propTypes no longer supported in React Native 0.58 raynor85/react-native-modal-wrapper#14

Closed

facebook locked as resolved and limited conversation to collaborators Oct 16, 2019

react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing propTypes from core components #21342

Removing propTypes from core components #21342

TheSavior commented Sep 26, 2018 •

edited

empyrical commented Sep 26, 2018 •

edited

richardcann commented Sep 27, 2018

TheSavior commented Sep 27, 2018

empyrical commented Sep 27, 2018

richardcann commented Sep 27, 2018

kdastan commented Sep 27, 2018

nissy-dev commented Sep 27, 2018

peaonunes commented Sep 27, 2018

TheSavior commented Sep 27, 2018

danibonilha commented Sep 27, 2018 •

edited

empyrical commented Sep 27, 2018

danibonilha commented Sep 27, 2018

exced commented Sep 27, 2018 •

edited

empyrical commented Sep 27, 2018

douglasjunior commented Jan 28, 2019

TheSavior commented Jan 28, 2019

TheSavior commented Jan 21, 2022

Removing propTypes from core components #21342

Removing propTypes from core components #21342

Comments

TheSavior commented Sep 26, 2018 • edited

Remove callsites from components:

Move and Rename custom propType definitions

empyrical commented Sep 26, 2018 • edited

Tips for removing propTypes from components

richardcann commented Sep 27, 2018

TheSavior commented Sep 27, 2018

empyrical commented Sep 27, 2018

richardcann commented Sep 27, 2018

kdastan commented Sep 27, 2018

nissy-dev commented Sep 27, 2018

peaonunes commented Sep 27, 2018

TheSavior commented Sep 27, 2018

danibonilha commented Sep 27, 2018 • edited

empyrical commented Sep 27, 2018

danibonilha commented Sep 27, 2018

exced commented Sep 27, 2018 • edited

empyrical commented Sep 27, 2018

douglasjunior commented Jan 28, 2019

TheSavior commented Jan 28, 2019

TheSavior commented Jan 21, 2022

TheSavior commented Sep 26, 2018 •

edited

Move and Rename custom `propType` definitions

empyrical commented Sep 26, 2018 •

edited

danibonilha commented Sep 27, 2018 •

edited

exced commented Sep 27, 2018 •

edited