Skip to content

Commit f39d44e

Browse files
committed
common/Input: Avoid not-recommended way of using react-intl API.
Following from discussion on GitHub [1]. We would have a bug if multiple chunks were passed to the render callback prop [2], since we're just using the first one. It looks like multiple chunks won't be passed, since we haven't explicitly made that happen. Glad to have this API misuse exposed by the API change in `react-intl` v5, which we took in a recent commit: "`FormattedMessage` render prop is no longer variadic". But we don't need to use this render prop pattern at all. From the docs [3]: """ If you need to customize rendering, you can either [...] or pass a function as the child. """ We just need plain old strings, without customization. So, just do the more straightforward thing we do in a bunch of places: use `formatMessage` as it's wrapped by `_`. Plumb through `_` using `withGetText` as recommended by the JSDoc for `GetText`: """ Alternatively, for when `context` is already in use: use `withGetText` and then say `const { _ } = this.props`. """ [1] #4222 (comment) [2] The render callback prop's name is `children`. The "Function as Child Component" pattern is sometimes opposed because `children` is rarely an appropriate, expressive name; see, e.g., https://americanexpress.io/faccs-are-an-antipattern/. Here, it also doesn't seem to be well-typed in the libdef. I'm unsure if the "FaCC" pattern makes it especially hard to get well-typed, but it seems plausible. [3] #4222 (comment)
1 parent 8fb9798 commit f39d44e

File tree

3 files changed

+25
-29
lines changed

3 files changed

+25
-29
lines changed

src/common/Input.js

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
/* @flow strict-local */
22
import React, { PureComponent } from 'react';
33
import { TextInput, Platform } from 'react-native';
4-
import { FormattedMessage } from 'react-intl';
54

6-
import type { LocalizableText } from '../types';
5+
import type { LocalizableText, GetText } from '../types';
76
import type { ThemeData } from '../styles';
87
import { ThemeContext, HALF_COLOR, BORDER_COLOR } from '../styles';
8+
import { withGetText } from '../boot/TranslationProvider';
99

1010
export type Props = $ReadOnly<{|
1111
...$PropertyType<TextInput, 'props'>,
1212
placeholder: LocalizableText,
1313
onChangeText?: (text: string) => void,
1414
textInputRef?: (component: ?TextInput) => void,
15+
16+
_: GetText,
1517
|}>;
1618

1719
type State = {|
@@ -32,7 +34,7 @@ type State = {|
3234
* @prop ...all other TextInput props - Passed through verbatim to TextInput.
3335
* See upstream: https://reactnative.dev/docs/textinput
3436
*/
35-
export default class Input extends PureComponent<Props, State> {
37+
class Input extends PureComponent<Props, State> {
3638
static contextType = ThemeContext;
3739
context: ThemeData;
3840

@@ -77,37 +79,31 @@ export default class Input extends PureComponent<Props, State> {
7779
};
7880

7981
render() {
80-
const { style, placeholder, textInputRef, ...restProps } = this.props;
82+
const { style, placeholder, textInputRef, _, ...restProps } = this.props;
8183
const { isFocused } = this.state;
8284
const fullPlaceholder =
8385
typeof placeholder === 'object' /* force linebreak */
8486
? placeholder
8587
: { text: placeholder, values: undefined };
8688

8789
return (
88-
<FormattedMessage
89-
id={fullPlaceholder.text}
90-
defaultMessage={fullPlaceholder.text}
91-
values={fullPlaceholder.values}
92-
>
93-
{(chunks: string[]) => (
94-
<TextInput
95-
style={[this.styles.input, { color: this.context.color }, style]}
96-
placeholder={chunks[0]}
97-
placeholderTextColor={HALF_COLOR}
98-
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
99-
onFocus={this.handleFocus}
100-
onBlur={this.handleBlur}
101-
ref={(component: ?TextInput) => {
102-
this.textInput = component;
103-
if (textInputRef) {
104-
textInputRef(component);
105-
}
106-
}}
107-
{...restProps}
108-
/>
109-
)}
110-
</FormattedMessage>
90+
<TextInput
91+
style={[this.styles.input, { color: this.context.color }, style]}
92+
placeholder={_(fullPlaceholder.text, fullPlaceholder.values)}
93+
placeholderTextColor={HALF_COLOR}
94+
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
95+
onFocus={this.handleFocus}
96+
onBlur={this.handleBlur}
97+
ref={(component: ?TextInput) => {
98+
this.textInput = component;
99+
if (textInputRef) {
100+
textInputRef(component);
101+
}
102+
}}
103+
{...restProps}
104+
/>
111105
);
112106
}
113107
}
108+
109+
export default withGetText(Input);

src/common/InputWithClearButton.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const componentStyles = createStyleSheet({
1414
},
1515
});
1616

17-
type Props = $ReadOnly<$Diff<InputProps, { textInputRef: mixed, value: mixed }>>;
17+
type Props = $ReadOnly<$Diff<InputProps, { textInputRef: mixed, value: mixed, _: mixed }>>;
1818

1919
type State = {|
2020
canBeCleared: boolean,

src/common/PasswordInput.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const styles = createStyleSheet({
2727
type Props = $ReadOnly<$Diff<InputProps,
2828
// "mixed" here is a way of spelling "no matter *what* type
2929
// `InputProps` allows for these, don't allow them here."
30-
{ secureTextEntry: mixed, autoCorrect: mixed, autoCapitalize: mixed }>>;
30+
{ secureTextEntry: mixed, autoCorrect: mixed, autoCapitalize: mixed, _: mixed }>>;
3131

3232
type State = {|
3333
isHidden: boolean,

0 commit comments

Comments
 (0)