Skip to content
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

[flow-strict] Flow strict in Picker.js, PickerIOS.ios.js, PickerAndroid.android.js #22128

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Libraries/Components/Picker/Picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type PickerItemProps = $ReadOnly<{|
* The value to be passed to picker's `onValueChange` callback when
* this item is selected. Can be a string or an integer.
*/
value?: any,
value?: ?(number | string),

/**
* Color of this item's text.
Expand Down Expand Up @@ -63,14 +63,14 @@ type PickerProps = $ReadOnly<{|
/**
* Value matching value of one of the items. Can be a string or an integer.
*/
selectedValue?: any,
selectedValue?: ?(number | string),

/**
* Callback for when an item is selected. This is called with the following parameters:
* - `itemValue`: the `value` prop of the item that was selected
* - `itemPosition`: the index of the selected item in this picker
* - `itemIndex`: the index of the selected item in this picker
*/
onValueChange?: ?(newValue: any, newIndex: number) => mixed,
onValueChange?: ?(itemValue: string | number, itemIndex: number) => mixed,

/**
* If set to false, the picker will be disabled, i.e. the user will not be able to make a
Expand Down
24 changes: 20 additions & 4 deletions Libraries/Components/Picker/PickerAndroid.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow
* @flow strict-local
*/

'use strict';
Expand Down Expand Up @@ -34,18 +34,34 @@ type PickerAndroidChangeEvent = SyntheticEvent<
type PickerAndroidProps = $ReadOnly<{|
children?: React.Node,
style?: ?TextStyleProp,
selectedValue?: any,
selectedValue?: ?(number | string),
enabled?: ?boolean,
mode?: ?('dialog' | 'dropdown'),
onValueChange?: ?(newValue: any, newIndex: number) => mixed,
onValueChange?: ?(itemValue: ?(string | number), itemIndex: number) => mixed,
prompt?: ?string,
testID?: string,
|}>;

type Item = $ReadOnly<{|
label: string,
value: ?(number | string),
color?: ?number,
|}>;

type PickerAndroidState = {|
initialSelectedIndex: number,
selectedIndex: number,
items: $ReadOnlyArray<Item>,
|};

/**
* Not exposed as a public API - use <Picker> instead.
*/
class PickerAndroid extends React.Component<PickerAndroidProps, *> {

class PickerAndroid extends React.Component<
PickerAndroidProps,
PickerAndroidState,
> {
/* $FlowFixMe(>=0.78.0 site=react_native_android_fb) This issue was found
* when making Flow check .android.js files. */
constructor(props, context) {
Expand Down
10 changes: 5 additions & 5 deletions Libraries/Components/Picker/PickerIOS.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ import type {TextStyleProp} from 'StyleSheet';

type PickerIOSChangeEvent = SyntheticEvent<
$ReadOnly<{|
newValue: any,
newValue: number | string,
newIndex: number,
|}>,
>;

type RCTPickerIOSItemType = $ReadOnly<{|
label: ?Label,
value: ?any,
value: ?(number | string),
textColor: ?number,
|}>;

Expand Down Expand Up @@ -62,8 +62,8 @@ type Props = $ReadOnly<{|
children: React.ChildrenArray<React.Element<typeof PickerIOSItem>>,
itemStyle?: ?TextStyleProp,
onChange?: ?(event: PickerIOSChangeEvent) => mixed,
onValueChange?: ?(newValue: any, newIndex: number) => mixed,
selectedValue: any,
onValueChange?: ?(itemValue: string | number, itemIndex: number) => mixed,
selectedValue: ?(number | string),
|}>;

type State = {|
Expand All @@ -73,7 +73,7 @@ type State = {|

type ItemProps = $ReadOnly<{|
label: ?Label,
value?: ?any,
value?: ?(number | string),
color?: ?ColorValue,
|}>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ describe('deepFreezeAndThrowOnMutationInDev', function() {
expect(o.key1.key2).toBe('newValue');
});

it('shouldn\'t recurse infinitely', () => {
it("shouldn't recurse infinitely", () => {
__DEV__ = true;
const o = {};
o.circular = o;
Expand Down
2 changes: 1 addition & 1 deletion RNTester/js/PermissionsExampleAndroid.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class PermissionsExample extends React.Component<{}, $FlowFixMeState> {
);
}

_onSelectPermission = (permission: string) => {
_onSelectPermission = (permission: string | number) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fix is not needed.
But flow needs... is this correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure without trying to test it out myself. What flow error do you get without this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get this error...

Cannot create Picker element because string [1] is incompatible with number [2] in the
first argument of property onValueChange.

     RNTester/js/PermissionsExampleAndroid.android.js
     38     return (
     39       <View style={styles.container}>
     40         <Text style={styles.text}>Permission Name:</Text>
     41         <Picker
     42           style={styles.picker}
     43           selectedValue={this.state.permission}
                    // this line causes error
     44           onValueChange={this._onSelectPermission.bind(this)}>
     45           <Item
     46             label={PermissionsAndroid.PERMISSIONS.CAMERA}
     47             value={PermissionsAndroid.PERMISSIONS.CAMERA}
     48           />
     49           <Item
     50             label={PermissionsAndroid.PERMISSIONS.READ_CALENDAR}
     51             value={PermissionsAndroid.PERMISSIONS.READ_CALENDAR}
     52           />
     53           <Item
     54             label={PermissionsAndroid.PERMISSIONS.ACCESS_FINE_LOCATION}
     55             value={PermissionsAndroid.PERMISSIONS.ACCESS_FINE_LOCATION}
     56           />
     57         </Picker>
     58         <TouchableWithoutFeedback onPress={this._checkPermission}>
     59│           <View>
     60│             <Text style={[styles.touchable, styles.text]}>
       :
 [1] 79│   _onSelectPermission = (permission: string) => {

this.setState({
permission: permission,
});
Expand Down
2 changes: 1 addition & 1 deletion RNTester/js/PickerExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class PickerExample extends React.Component<{}, $FlowFixMeState> {
this.setState({mode: newMode});
};

onValueChange = (key: string, value: string) => {
onValueChange = (key: string, value: string | number) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fix is not needed.
But flow needs... is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get this error....

Cannot create Picker element because string [1] is incompatible with number [2] in the
first argument of property onValueChange.

     RNTester/js/PickerExample.js
      37     return (
      38       <RNTesterPage title="<Picker>">
      39         <RNTesterBlock title="Basic Picker">
      40           <Picker
      41             style={styles.picker}
      42             selectedValue={this.state.selected1}
                      // this line causes error
      43             onValueChange={this.onValueChange.bind(this, 'selected1')}>
      44             <Item label="hello" value="key0" />
      45             <Item label="world" value="key1" />
      46           </Picker>
      47         </RNTesterBlock>
      48         <RNTesterBlock title="Disabled picker">
      49│           <Picker
        :
 [1] 119   onValueChange = (key: string, value: string) => {

     Libraries/Components/Picker/Picker.js
 [2]  73   onValueChange?: ?(itemValue: string | number, itemIndex: number) => mixed,

const newState = {};
newState[key] = value;
this.setState(newState);
Expand Down