Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[No QA] No Props/State Destructuring #6234

Merged
merged 10 commits into from
Nov 8, 2021
22 changes: 14 additions & 8 deletions STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,7 @@ const {name, accountID, email} = data;

**React Components**

- Avoid destructuring props and state at the *same time*. It makes the source of a given variable unclear.
- Avoid destructuring either props or state when there are other variables declared in a render block. This helps us quickly know which variables are from props, state, or declared inside the render.
- Use parameter destructuring for stateless function components when there are no additional variable declarations in the render.
Don't destructure props or state. It makes the source of a given variable unclear. This guideline helps us quickly know which variables are from props, state, or from some other scope.

```javascript
// Bad
Expand All @@ -276,12 +274,20 @@ render() {
...
}

// Good
// Bad
const UserInfo = ({name, email}) => (
<div>
<p>Name: {name}</p>
<p>Email: {email}</p>
</div>
<View>
<Text>Name: {name}</Text>
<Text>Email: {email}</Text>
</View>
);

// Good
const UserInfo = props => (
<View>
<Text>Name: {props.name}</Text>
<Text>Email: {props.email}</Text>
</View>
);
```

Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
"electron-notarize": "^1.0.0",
"electron-reloader": "^1.2.0",
"eslint": "^7.6.0",
"eslint-config-expensify": "2.0.17",
"eslint-config-expensify": "^2.0.18",
"eslint-loader": "^4.0.2",
"eslint-plugin-detox": "^1.0.0",
"eslint-plugin-jest": "^24.1.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React from 'react';
import {Pressable, StyleSheet} from 'react-native';
import lodashGet from 'lodash/get';
Expand All @@ -9,31 +10,22 @@ import {CONTEXT_MENU_TYPES} from '../../../pages/home/report/ContextMenu/Context
import AttachmentView from '../../AttachmentView';
import fileDownload from '../../../libs/fileDownload';


/*
* This is a default anchor component for regular links.
*/
const BaseAnchorForCommentsOnly = ({
href,
rel,
target,
children,
style,
fileName,
...props
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
}) => {
const BaseAnchorForCommentsOnly = (props) => {
let linkRef;
const rest = _.omit(props, _.keys(propTypes));
return (

props.isAttachment
? (
<Pressable onPress={() => {
fileDownload(href, fileName);
fileDownload(props.href, props.fileName);
}}
>
<AttachmentView
sourceURL={href}
file={{name: fileName}}
sourceURL={props.href}
file={{name: props.fileName}}
shouldShowDownloadIcon
/>
</Pressable>
Expand All @@ -45,22 +37,25 @@ const BaseAnchorForCommentsOnly = ({
showContextMenu(
CONTEXT_MENU_TYPES.LINK,
event,
href,
props.href,
lodashGet(linkRef, 'current'),
);
}
}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(style)}
style={StyleSheet.flatten(props.style)}
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
accessibilityRole="link"
href={href}
hrefAttrs={{rel, target}}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
href={props.href}
hrefAttrs={{
rel: props.rel,
target: props.target,
}}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{children}
{props.children}
</Text>
</PressableWithSecondaryInteraction>
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React from 'react';
import lodashGet from 'lodash/get';
import {Linking, StyleSheet, Pressable} from 'react-native';
Expand All @@ -13,27 +14,21 @@ import styles from '../../../styles/styles';
/*
* This is a default anchor component for regular links.
*/
const BaseAnchorForCommentsOnly = ({
href,
children,
style,
isAttachment,
fileName,
...props
}) => {
const BaseAnchorForCommentsOnly = (props) => {
let linkRef;
const rest = _.omit(props, _.keys(propTypes));
return (
isAttachment
props.isAttachment
? (
<Pressable
style={styles.mw100}
onPress={() => {
fileDownload(href, fileName);
fileDownload(props.href, props.fileName);
}}
>
<AttachmentView
sourceURL={href}
file={{name: fileName}}
sourceURL={props.href}
file={{name: props.fileName}}
shouldShowDownloadIcon
/>
</Pressable>
Expand All @@ -45,20 +40,20 @@ const BaseAnchorForCommentsOnly = ({
showContextMenu(
CONTEXT_MENU_TYPES.LINK,
event,
href,
props.href,
lodashGet(linkRef, 'current'),
);
}
}
onPress={() => Linking.openURL(href)}
onPress={() => Linking.openURL(props.href)}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(style)}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
style={StyleSheet.flatten(props.style)}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{children}
{props.children}
</Text>
</PressableWithSecondaryInteraction>
)
Expand Down
2 changes: 1 addition & 1 deletion src/components/AvatarWithImagePicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class AvatarWithImagePicker extends React.Component {
}

render() {
const {DefaultAvatar} = this.props;
const DefaultAvatar = this.props.DefaultAvatar;
const additionalStyles = _.isArray(this.props.style) ? this.props.style : [this.props.style];

const indicatorStyles = [
Expand Down
4 changes: 2 additions & 2 deletions src/components/BigNumberPad.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const padNumbers = [
['.', '0', '<'],
];

const BigNumberPad = ({numberPressed}) => (
const BigNumberPad = props => (
<View style={[styles.flexColumn, styles.w100]}>
{_.map(padNumbers, (row, rowIndex) => (
<View key={`NumberPadRow-${rowIndex}`} style={[styles.flexRow, styles.mt3]}>
Expand All @@ -30,7 +30,7 @@ const BigNumberPad = ({numberPressed}) => (
key={column}
style={[styles.flex1, marginLeft]}
text={column}
onPress={() => numberPressed(column)}
onPress={() => props.numberPressed(column)}
/>
);
})}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class Button extends Component {
}

renderContent() {
const {ContentComponent} = this.props;
const ContentComponent = this.props.ContentComponent;
if (ContentComponent) {
return <ContentComponent />;
}
Expand Down
17 changes: 6 additions & 11 deletions src/components/Checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,17 @@ const defaultProps = {
disabled: false,
};

const Checkbox = ({
isChecked,
onPress,
hasError,
disabled,
}) => (
const Checkbox = props => (
<Pressable
disabled={disabled}
onPress={() => onPress(!isChecked)}
disabled={props.disabled}
onPress={() => props.onPress(!props.isChecked)}
>
<View
style={[
styles.checkboxContainer,
isChecked && styles.checkedContainer,
hasError && styles.borderColorDanger,
disabled && styles.cursorDisabled,
props.isChecked && styles.checkedContainer,
props.hasError && styles.borderColorDanger,
props.disabled && styles.cursorDisabled,
]}
>
<Icon src={Checkmark} fill="white" height={14} width={14} />
Expand Down
27 changes: 13 additions & 14 deletions src/components/CheckboxWithLabel.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,25 @@ const defaultProps = {
errorText: '',
};

const CheckboxWithLabel = ({
LabelComponent, isChecked, onPress, style, label, hasError, errorText,
}) => {
const CheckboxWithLabel = (props) => {
const LabelComponent = props.LabelComponent;
const defaultStyles = [styles.flexRow, styles.alignItemsCenter];
const wrapperStyles = _.isArray(style) ? [...defaultStyles, ...style] : [...defaultStyles, style];
const wrapperStyles = _.isArray(props.style) ? [...defaultStyles, ...props.style] : [...defaultStyles, props.style];

if (!label && !LabelComponent) {
if (!props.label && !LabelComponent) {
throw new Error('Must provide at least label or LabelComponent prop');
}
return (
<>
<View style={wrapperStyles}>
<Checkbox
isChecked={isChecked}
onPress={() => onPress(!isChecked)}
label={label}
hasError={hasError}
isChecked={props.isChecked}
onPress={() => props.onPress(!props.isChecked)}
label={props.label}
hasError={props.hasError}
/>
<TouchableOpacity
onPress={() => onPress(!isChecked)}
onPress={() => props.onPress(!props.isChecked)}
style={[
styles.ml3,
styles.pr2,
Expand All @@ -68,17 +67,17 @@ const CheckboxWithLabel = ({
styles.alignItemsCenter,
]}
>
{label && (
{props.label && (
<Text style={[styles.ml2]}>
{label}
{props.label}
</Text>
)}
{LabelComponent && (<LabelComponent />)}
</TouchableOpacity>
</View>
{!_.isEmpty(errorText) && (
{!_.isEmpty(props.errorText) && (
<InlineErrorText>
{errorText}
{props.errorText}
</InlineErrorText>
)}
</>
Expand Down
29 changes: 9 additions & 20 deletions src/components/DatePicker/index.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,36 +38,25 @@ class DatePicker extends React.Component {
}

render() {
const {
value,
label,
placeholder,
hasError,
errorText,
translateX,
containerStyles,
disabled,
} = this.props;

const dateAsText = value ? moment(value).format(CONST.DATE.MOMENT_FORMAT_STRING) : '';
const dateAsText = this.props.value ? moment(this.props.value).format(CONST.DATE.MOMENT_FORMAT_STRING) : '';

return (
<>
<ExpensiTextInput
label={label}
label={this.props.label}
value={dateAsText}
placeholder={placeholder}
hasError={hasError}
errorText={errorText}
containerStyles={containerStyles}
translateX={translateX}
placeholder={this.props.placeholder}
hasError={this.props.hasError}
errorText={this.props.errorText}
containerStyles={this.props.containerStyles}
translateX={this.props.translateX}
onPress={this.showPicker}
editable={false}
disabled={disabled}
disabled={this.props.disabled}
/>
{this.state.isPickerVisible && (
<RNDatePicker
value={value ? moment(value).toDate() : new Date()}
value={this.props.value ? moment(this.props.value).toDate() : new Date()}
mode="date"
onChange={this.raiseDateChange}
/>
Expand Down
Loading