TextInput: Remove PropTypes, NativeMethodsMixin; Convert to ES6 class

[empyrical]

This pull requests converts TextInput to an ES6 class, and in the process removes its usage of prop-types and NativeMethodsMixin.

The code (and some relevant types) for the native components have been moved to TextInputNativeComponent.js.

The rest of the flow proptypes have been moved to TextInputTypes.js.

Test Plan:

These have both been tried in RNTester, and no behaviour regression has been observed.

Release Notes:

Help reviewers and the release process by writing your own release notes. See below for an example.

[GENERAL] [ENHANCEMENT] [Libraries/Components/TextInput/TextInput.js] - Removed Mixin and PropTypes, converted to ES6 class
[GENERAL] [ENHANCEMENT] [Libraries/Components/TextInput/TextInputNativeComponent.js] - Moved native component importing logic to its own file
[GENERAL] [ENHANCEMENT] [Libraries/Components/TextInput/TextInputTypes.js] - Moved Flow types to their own file

[empyrical]

I would like to fill out the different event types (and the callbacks that use them) but would like to do that in a separate diff

[elicwhite]

Can you at least create different types for the events? You can keep them all as Object for now but it will at least help knowing what the different types should be and that they aren't necessarily all the same shape.

[elicwhite]

What do you think about doing the casting to Class<TextInputType> here instead?

[elicwhite]

Woah, I had no idea.

[elicwhite]

Maybe we should nullthrows this one as well to avoid changing the logic.

[elicwhite]

We still need this in order to have context passed in, right? Interestingly, I don't see any reference to either of these context variables internally and I believe they were part of the initial react sync.

I'd like to avoid changing logic as much as possible so maybe we should keep these in this PR and remove reference to them in a separate one.

[elicwhite]

In the other examples in the repo it seems like it is typed as:

forwardedRef: ?React.Ref<'RCTImageView'>,

Or

forwardedRef: ?React.Ref<'RCTTextInlineImage' | 'ImageViewNativeComponent'>,

Should we do the same thing here?

[empyrical]

When I typed it with a string like that, it seemed to interpret it as a ref to a DOM element. I think those sites have a FlowFixMe that suppresses type errors

[facebook-github-bot]

TheSavior has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[elicwhite]

I'm currently stuck on some internal Jest tests which are triggering the TextInput's _onChange function via:

const {root} = ReactTestRenderer.create(<MyComponent />);
const instance = root.findByProps({ testID: 'my_test_id' });
const input = instance.findByType(TextInput);
input.instance._onChange({nativeEvent: {text}});

Jest's setup file is calling:

jest.mock('TextInput', () => mockComponent('TextInput'))

When migrating to forwardRef, TextInput is now the forwardRef function and not the underlying class so the mock isn't valid.

I was thinking that maybe the easiest thing to do is just move the class into its own file like TextInputInternal.js and change to mocking that file but it seems like a lot of leaking internals. Does anyone have an idea how to work around this?

[rajivshah3]

@TheSavior I might be misunderstanding, but is it not finding the TextInput because it now uses forwardRef? If so, maybe this could be fixed by adding the displayName like #21950 did for View and Text

[elicwhite]

Whoops, I should have responded here that I found a way to get past this but then I got stuck on something else and haven’t been able to look at this for the last few days. Haha.

I should be able to get back to this early next week or @RSNara can help get this one through internally in the mean time.

[react-native-bot]

@empyrical merged commit 70e9e26 into facebook:master.