Skip to content

Commit ee41e4c

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 57fa649 commit ee41e4c

File tree

3 files changed

+20
-24
lines changed

3 files changed

+20
-24
lines changed

src/common/Input.js

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
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
// Should be fixed in RN v0.63 (#4245); see
@@ -15,6 +15,8 @@ export type Props = $ReadOnly<{|
1515
placeholder: LocalizableText,
1616
onChangeText?: (text: string) => void,
1717
textInputRef?: React$Ref<typeof TextInput>,
18+
19+
_: GetText,
1820
|}>;
1921

2022
type State = {|
@@ -35,7 +37,7 @@ type State = {|
3537
* @prop ...all other TextInput props - Passed through verbatim to TextInput.
3638
* See upstream: https://reactnative.dev/docs/textinput
3739
*/
38-
export default class Input extends PureComponent<Props, State> {
40+
class Input extends PureComponent<Props, State> {
3941
static contextType = ThemeContext;
4042
context: ThemeData;
4143

@@ -69,32 +71,26 @@ export default class Input extends PureComponent<Props, State> {
6971
};
7072

7173
render() {
72-
const { style, placeholder, textInputRef, ...restProps } = this.props;
74+
const { style, placeholder, textInputRef, _, ...restProps } = this.props;
7375
const { isFocused } = this.state;
7476
const fullPlaceholder =
7577
typeof placeholder === 'object' /* force linebreak */
7678
? placeholder
7779
: { text: placeholder, values: undefined };
7880

7981
return (
80-
<FormattedMessage
81-
id={fullPlaceholder.text}
82-
defaultMessage={fullPlaceholder.text}
83-
values={fullPlaceholder.values}
84-
>
85-
{(chunks: string[]) => (
86-
<TextInput
87-
style={[this.styles.input, { color: this.context.color }, style]}
88-
placeholder={chunks[0]}
89-
placeholderTextColor={HALF_COLOR}
90-
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
91-
onFocus={this.handleFocus}
92-
onBlur={this.handleBlur}
93-
ref={textInputRef}
94-
{...restProps}
95-
/>
96-
)}
97-
</FormattedMessage>
82+
<TextInput
83+
style={[this.styles.input, { color: this.context.color }, style]}
84+
placeholder={_(fullPlaceholder.text, fullPlaceholder.values)}
85+
placeholderTextColor={HALF_COLOR}
86+
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
87+
onFocus={this.handleFocus}
88+
onBlur={this.handleBlur}
89+
ref={textInputRef}
90+
{...restProps}
91+
/>
9892
);
9993
}
10094
}
95+
96+
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)