Skip to content

Commit

Permalink
Convert to using forwardRef
Browse files Browse the repository at this point in the history 
Summary:
TextInput now acts as a host component and can be passed directly to our new APIs that require a host component. Callsites no longer need to call

```
inputRef.getNativeRef()
```

We mutate the ref to the host component adding the imperative methods of the TextInput so you can still call `inputRef.clear` and `inputRef.isFocused`.

Changelog:
[General][Changed] TextInput now uses `forwardRef` allowing it to be used directly by new APIs requiring a host component.

Reviewed By: yungsters

Differential Revision: D18458408

fbshipit-source-id: 1f149fd575210d702fa0fdf3d05bb2162436a773
  • Loading branch information
@TheSavior @facebook-github-bot
TheSavior authored and facebook-github-bot committed Nov 15, 2019
1 parent 99dc4e2 commit bbc5c35
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 81 deletions.
172 changes: 105 additions & 67 deletions Libraries/Components/TextInput/TextInput.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,18 @@ const TextInputState = require('./TextInputState');
const TouchableWithoutFeedback = require('../Touchable/TouchableWithoutFeedback');

const invariant = require('invariant');
const nullthrows = require('nullthrows');
const requireNativeComponent = require('../../ReactNative/requireNativeComponent');
const setAndForwardRef = require('../../Utilities/setAndForwardRef');

import type {TextStyleProp, ViewStyleProp} from '../../StyleSheet/StyleSheet';
import type {ColorValue} from '../../StyleSheet/StyleSheetTypes';
import type {ViewProps} from '../View/ViewPropTypes';
import type {SyntheticEvent, ScrollEvent} from '../../Types/CoreEventTypes';
import type {PressEvent} from '../../Types/CoreEventTypes';
import type {
HostComponent,
MeasureOnSuccessCallback,
MeasureInWindowOnSuccessCallback,
MeasureLayoutOnSuccessCallback,
} from '../../Renderer/shims/ReactNativeTypes';
import type {HostComponent} from '../../Renderer/shims/ReactNativeTypes';

type ReactRefSetter<T> = {current: null | T} | ((ref: null | T) => mixed);

let AndroidTextInput;
let RCTMultilineTextInputView;
Expand Down Expand Up @@ -676,6 +675,10 @@ export type Props = $ReadOnly<{|
* If `true`, contextMenuHidden is hidden. The default value is `false`.
*/
contextMenuHidden?: ?boolean,

forwardedRef?: ?ReactRefSetter<
React.ElementRef<HostComponent<mixed>> & ImperativeMethods,
>,
|}>;

type DefaultProps = $ReadOnly<{|
Expand All @@ -684,11 +687,11 @@ type DefaultProps = $ReadOnly<{|
underlineColorAndroid: 'transparent',
|}>;

type State = {|
currentlyFocusedField: typeof TextInputState.currentlyFocusedField,
focusTextInput: typeof TextInputState.focusTextInput,
blurTextInput: typeof TextInputState.blurTextInput,
|};
type ImperativeMethods = $ReadOnly<{|
clear: () => void,
isFocused: () => boolean,
getNativeRef: () => ?React.ElementRef<HostComponent<mixed>>,
|}>;

const emptyFunctionThatReturnsTrue = () => true;

Expand Down Expand Up @@ -803,22 +806,14 @@ const emptyFunctionThatReturnsTrue = () => true;
* or control this param programmatically with native code.
*
*/
class TextInput extends React.Component<Props, State> {
class InternalTextInput extends React.Component<Props> {
static defaultProps: DefaultProps = {
allowFontScaling: true,
rejectResponderTermination: true,
underlineColorAndroid: 'transparent',
};

static propTypes = DeprecatedTextInputPropTypes;

static State: State = {
currentlyFocusedField: TextInputState.currentlyFocusedField,
focusTextInput: TextInputState.focusTextInput,
blurTextInput: TextInputState.blurTextInput,
};

_inputRef: ?React.ElementRef<HostComponent<mixed>> = null;
_inputRef: null | React.ElementRef<HostComponent<mixed>> = null;
_focusSubscription: ?Function = undefined;
_lastNativeText: ?Stringish = null;
_lastNativeSelection: ?Selection = null;
Expand All @@ -833,7 +828,11 @@ class TextInput extends React.Component<Props, State> {
}

if (this.props.autoFocus) {
this._rafId = requestAnimationFrame(this.focus);
this._rafId = requestAnimationFrame(() => {
if (this._inputRef) {
this._inputRef.focus();
}
});
}
}

Expand Down Expand Up @@ -874,7 +873,7 @@ class TextInput extends React.Component<Props, State> {
componentWillUnmount() {
this._focusSubscription && this._focusSubscription.remove();
if (this.isFocused()) {
this.blur();
nullthrows(this._inputRef).blur();
}
const tag = ReactNative.findNodeHandle(this._inputRef);
if (tag != null) {
Expand All @@ -889,7 +888,9 @@ class TextInput extends React.Component<Props, State> {
* Removes all text from the `TextInput`.
*/
clear: () => void = () => {
this.setNativeProps({text: ''});
if (this._inputRef != null) {
this._inputRef.setNativeProps({text: ''});
}
};

/**
Expand All @@ -906,35 +907,6 @@ class TextInput extends React.Component<Props, State> {
return this._inputRef;
};

// From NativeMethodsMixin
// We need these instead of using forwardRef because we also have the other
// methods we expose
blur: () => void = () => {
this._inputRef && this._inputRef.blur();
};
focus: () => void = () => {
this._inputRef && this._inputRef.focus();
};
measure: (callback: MeasureOnSuccessCallback) => void = callback => {
this._inputRef && this._inputRef.measure(callback);
};
measureInWindow: (
callback: MeasureInWindowOnSuccessCallback,
) => void = callback => {
this._inputRef && this._inputRef.measureInWindow(callback);
};
measureLayout: (
relativeToNativeNode: number | React.ElementRef<HostComponent<mixed>>,
onSuccess: MeasureLayoutOnSuccessCallback,
onFail?: () => void,
) => void = (relativeToNativeNode, onSuccess, onFail) => {
this._inputRef &&
this._inputRef.measureLayout(relativeToNativeNode, onSuccess, onFail);
};
setNativeProps: (nativeProps: Object) => void = nativeProps => {
this._inputRef && this._inputRef.setNativeProps(nativeProps);
};

render(): React.Node {
let textInput = null;
let additionalTouchableProps: {|
Expand Down Expand Up @@ -1045,13 +1017,44 @@ class TextInput extends React.Component<Props, State> {
: '';
}

_setNativeRef = (ref: any) => {
this._inputRef = ref;
};
_setNativeRef = setAndForwardRef({
getForwardedRef: () => this.props.forwardedRef,
setLocalRef: ref => {
this._inputRef = ref;

/*
Hi reader from the future. I'm sorry for this.
This is a hack. Ideally we would forwardRef to the underlying
host component. However, since TextInput has it's own methods that can be
called as well, if we used the standard forwardRef then these
methods wouldn't be accessible and thus be a breaking change.
We have a couple of options of how to handle this:
- Return a new ref with everything we methods from both. This is problematic
because we need React to also know it is a host component which requires
internals of the class implementation of the ref.
- Break the API and have some other way to call one set of the methods or
the other. This is our long term approach as we want to eventually
get the methods on host components off the ref. So instead of calling
ref.measure() you might call ReactNative.measure(ref). This would hopefully
let the ref for TextInput then have the methods like `.clear`. Or we do it
the other way and make it TextInput.clear(textInputRef) which would be fine
too. Either way though is a breaking change that is longer term.
- Mutate this ref. :( Gross, but accomplishes what we need in the meantime
before we can get to the long term breaking change.
*/
if (ref) {
ref.clear = this.clear;
ref.isFocused = this.isFocused;
ref.getNativeRef = this.getNativeRef;
}
},
});

_onPress = (event: PressEvent) => {
if (this.props.editable || this.props.editable === undefined) {
this.focus();
nullthrows(this._inputRef).focus();
}
};

Expand Down Expand Up @@ -1117,16 +1120,44 @@ class TextInput extends React.Component<Props, State> {
};
}

class InternalTextInputType extends ReactNative.NativeComponent<Props> {
clear() {}

getNativeRef(): ?React.ElementRef<HostComponent<mixed>> {}

// $FlowFixMe
isFocused(): boolean {}
}
const ExportedForwardRef: React.AbstractComponent<
React.ElementConfig<typeof InternalTextInput>,
React.ElementRef<HostComponent<mixed>> & ImperativeMethods,
> = React.forwardRef(function TextInput(
props,
forwardedRef: ReactRefSetter<
React.ElementRef<HostComponent<mixed>> & ImperativeMethods,
>,
) {
return <InternalTextInput {...props} forwardedRef={forwardedRef} />;
});

const TypedTextInput = ((TextInput: any): Class<InternalTextInputType>);
// $FlowFixMe
ExportedForwardRef.defaultProps = {
allowFontScaling: true,
rejectResponderTermination: true,
underlineColorAndroid: 'transparent',
};

// TODO: Deprecate this
// $FlowFixMe
ExportedForwardRef.propTypes = DeprecatedTextInputPropTypes;

// $FlowFixMe
ExportedForwardRef.State = {
currentlyFocusedField: TextInputState.currentlyFocusedField,
focusTextInput: TextInputState.focusTextInput,
blurTextInput: TextInputState.blurTextInput,
};

type TextInputComponentStatics = $ReadOnly<{|
State: $ReadOnly<{|
currentlyFocusedField: typeof TextInputState.currentlyFocusedField,
focusTextInput: typeof TextInputState.focusTextInput,
blurTextInput: typeof TextInputState.blurTextInput,
|}>,
propTypes: typeof DeprecatedTextInputPropTypes,
|}>;

const styles = StyleSheet.create({
multilineInput: {
Expand All @@ -1137,4 +1168,11 @@ const styles = StyleSheet.create({
},
});

module.exports = TypedTextInput;
module.exports = ((ExportedForwardRef: any): React.AbstractComponent<
React.ElementConfig<typeof InternalTextInput>,
$ReadOnly<{|
...React.ElementRef<HostComponent<mixed>>,
...ImperativeMethods,
|}>,
> &
TextInputComponentStatics);
26 changes: 17 additions & 9 deletions Libraries/Components/TextInput/__tests__/TextInput-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const ReactTestRenderer = require('react-test-renderer');
const TextInput = require('../TextInput');
const ReactNative = require('../../../Renderer/shims/ReactNative');

import type {FocusEvent} from '../TextInput';
import Component from '@reactions/component';

const {enter} = require('../../../Utilities/ReactNativeTestTools');
Expand All @@ -25,16 +24,19 @@ jest.unmock('../TextInput');

describe('TextInput tests', () => {
let input;
let inputRef;
let onChangeListener;
let onChangeTextListener;
const initialValue = 'initialValue';
beforeEach(() => {
inputRef = React.createRef(null);
onChangeListener = jest.fn();
onChangeTextListener = jest.fn();
const renderTree = ReactTestRenderer.create(
<Component initialState={{text: initialValue}}>
{({setState, state}) => (
<TextInput
ref={inputRef}
value={state.text}
onChangeText={text => {
onChangeTextListener(text);
Expand All @@ -50,14 +52,20 @@ describe('TextInput tests', () => {
input = renderTree.root.findByType(TextInput);
});
it('has expected instance functions', () => {
expect(input.instance.isFocused).toBeInstanceOf(Function); // Would have prevented S168585
expect(input.instance.clear).toBeInstanceOf(Function);
expect(input.instance.focus).toBeInstanceOf(Function);
expect(input.instance.blur).toBeInstanceOf(Function);
expect(input.instance.setNativeProps).toBeInstanceOf(Function);
expect(input.instance.measure).toBeInstanceOf(Function);
expect(input.instance.measureInWindow).toBeInstanceOf(Function);
expect(input.instance.measureLayout).toBeInstanceOf(Function);
expect(inputRef.current.isFocused).toBeInstanceOf(Function); // Would have prevented S168585
expect(inputRef.current.clear).toBeInstanceOf(Function);
expect(inputRef.current.focus).toBeInstanceOf(jest.fn().constructor);
expect(inputRef.current.blur).toBeInstanceOf(jest.fn().constructor);
expect(inputRef.current.setNativeProps).toBeInstanceOf(
jest.fn().constructor,
);
expect(inputRef.current.measure).toBeInstanceOf(jest.fn().constructor);
expect(inputRef.current.measureInWindow).toBeInstanceOf(
jest.fn().constructor,
);
expect(inputRef.current.measureLayout).toBeInstanceOf(
jest.fn().constructor,
);
});
it('calls onChange callbacks', () => {
expect(input.props.value).toBe(initialValue);
Expand Down
5 changes: 1 addition & 4 deletions Libraries/Renderer/shims/ReactNativeTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,7 @@ export type NativeMethods = {
};

export type NativeMethodsMixinType = NativeMethods;
export type HostComponent<T> = AbstractComponent<
T,
$ReadOnly<$Exact<NativeMethods>>,
>;
export type HostComponent<T> = AbstractComponent<T, $ReadOnly<NativeMethods>>;

type SecretInternalsType = {
NativeMethodsMixin: NativeMethodsMixinType,
Expand Down
13 changes: 12 additions & 1 deletion jest/setup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ jest
mockComponent('../Libraries/Text/Text', MockNativeMethods),
)
.mock('../Libraries/Components/TextInput/TextInput', () =>
mockComponent('../Libraries/Components/TextInput/TextInput'),
mockComponent(
'../Libraries/Components/TextInput/TextInput',
MockNativeMethods,
),
)
.mock('../Libraries/Modal/Modal', () =>
mockComponent('../Libraries/Modal/Modal'),
Expand Down Expand Up @@ -312,6 +315,14 @@ jest
render() {
return React.createElement(viewName, this.props, this.props.children);
}

// The methods that exist on host components
blur = jest.fn();
focus = jest.fn();
measure = jest.fn();
measureInWindow = jest.fn();
measureLayout = jest.fn();
setNativeProps = jest.fn();
};

if (viewName === 'RCTView') {
Expand Down

2 comments on commit bbc5c35

@loubackrethink
Copy link

Choose a reason for hiding this comment

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

Nice, it was I'm looking for and I did similar in a project that I'm working...

@mattmogford
Copy link

Choose a reason for hiding this comment

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

I have a React.forwardRef(...) around functional component containing a TextInput, and passing it the ref.
Before From outside I would retrieve the value like so...
userIDRef.current?.props.value || ''
Since this update, props is undefined.
How do I now access the TextInput properties please?

I feel like this an easy fix, but a lot of my forms are broken with this update.

Thanks

Please sign in to comment.