Skip to content

Commit

Permalink
common/Input: Avoid not-recommended way of using react-intl API.
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
chrisbobbe authored and gnprice committed Oct 23, 2020
1 parent 31f2e3d commit 2734811
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 24 deletions.
40 changes: 18 additions & 22 deletions src/common/Input.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/* @flow strict-local */
import React, { PureComponent } from 'react';
import { TextInput, Platform } from 'react-native';
import { FormattedMessage } from 'react-intl';

import type { LocalizableText } from '../types';
import type { LocalizableText, GetText } from '../types';
import type { ThemeData } from '../styles';
import { ThemeContext, HALF_COLOR, BORDER_COLOR } from '../styles';
import { withGetText } from '../boot/TranslationProvider';

export type Props = $ReadOnly<{|
// Should be fixed in RN v0.63 (#4245); see
Expand All @@ -15,6 +15,8 @@ export type Props = $ReadOnly<{|
placeholder: LocalizableText,
onChangeText?: (text: string) => void,
textInputRef?: React$Ref<typeof TextInput>,

_: GetText,
|}>;

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

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

render() {
const { style, placeholder, textInputRef, ...restProps } = this.props;
const { style, placeholder, textInputRef, _, ...restProps } = this.props;
const { isFocused } = this.state;
const fullPlaceholder =
typeof placeholder === 'object' /* force linebreak */
? placeholder
: { text: placeholder, values: undefined };

return (
<FormattedMessage
id={fullPlaceholder.text}
defaultMessage={fullPlaceholder.text}
values={fullPlaceholder.values}
>
{(chunks: string[]) => (
<TextInput
style={[this.styles.input, { color: this.context.color }, style]}
placeholder={chunks[0]}
placeholderTextColor={HALF_COLOR}
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
onFocus={this.handleFocus}
onBlur={this.handleBlur}
ref={textInputRef}
{...restProps}
/>
)}
</FormattedMessage>
<TextInput
style={[this.styles.input, { color: this.context.color }, style]}
placeholder={_(fullPlaceholder.text, fullPlaceholder.values)}
placeholderTextColor={HALF_COLOR}
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
onFocus={this.handleFocus}
onBlur={this.handleBlur}
ref={textInputRef}
{...restProps}
/>
);
}
}

export default withGetText(Input);
2 changes: 1 addition & 1 deletion src/common/InputWithClearButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const componentStyles = createStyleSheet({
},
});

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

type State = {|
canBeCleared: boolean,
Expand Down
2 changes: 1 addition & 1 deletion src/common/PasswordInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const styles = createStyleSheet({
type Props = $ReadOnly<$Diff<InputProps,
// "mixed" here is a way of spelling "no matter *what* type
// `InputProps` allows for these, don't allow them here."
{ secureTextEntry: mixed, autoCorrect: mixed, autoCapitalize: mixed }>>;
{ secureTextEntry: mixed, autoCorrect: mixed, autoCapitalize: mixed, _: mixed }>>;

type State = {|
isHidden: boolean,
Expand Down

0 comments on commit 2734811

Please sign in to comment.