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] Fix all optional chains and nullish coalescing operator + update style guide #5988

Merged
merged 5 commits into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,23 @@ Empty functions (noop) should be declare as arrow functions with no whitespace i
}
```

## Accessing Object Properties and Default Values

Use `_.get()` to safely access object properties and `||` to short circuit null or undefined values that are not guaranteed to exist in a consistent way throughout the codebase. In the rare case that you want to consider a falsy value as usable and the `||` operator prevents this then be explicit about this in your code and check for the type using an underscore method e.g. `_.isBoolean(value)` or `_.isEqual(0)`.

```javascript
// Bad
const value = somePossiblyNullThing ?? 'default';
// Good
const value = somePossiblyNullThing || 'default';
// Bad
const value = someObject.possiblyUndefinedProperty?.nestedProperty || 'default';
// Bad
const value = (someObject && someObject.possiblyUndefinedProperty && someObject.possiblyUndefinedProperty.nestedProperty) || 'default';
// Good
const value = _.get(someObject, 'possiblyUndefinedProperty.nestedProperty', 'default');
```

## JSDocs
- Avoid docs that don't add any additional information.
- Always document parameters and return values.
Expand Down Expand Up @@ -292,7 +309,8 @@ So, if a new language feature isn't something we have agreed to support it's off
Here are a couple of things we would ask that you *avoid* to help maintain consistency in our codebase:

- **Async/Await** - Use the native `Promise` instead
- **Optional Chaining** - Use `lodash/get` to fetch a nested value instead
- **Optional Chaining** - Use `_.get()` to fetch a nested value instead
Copy link
Contributor

@aldo-expensify aldo-expensify Oct 22, 2021

Choose a reason for hiding this comment

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

Is _ underscore? or lodash? maybe we should clarify since we have both libraries or... we asume _ is underscore because that is how we normally import underscore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's underscore since whenever we use lodash we prefix with lodash*

- **Null Coalescing Operator** - Use `_.get()` or `||` to set a default value for a possibly `undefined` or `null` variable

# React Coding Standards

Expand Down
6 changes: 5 additions & 1 deletion src/components/AddressSearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ const defaultProps = {
const AddressSearch = (props) => {
const googlePlacesRef = useRef();
useEffect(() => {
googlePlacesRef.current?.setAddressText(props.value);
if (!googlePlacesRef.current) {
return;
}

googlePlacesRef.current.setAddressText(props.value);
}, []);

// eslint-disable-next-line
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Network.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ function processNetworkRequestQueue() {
}

const requestData = queuedRequest.data;
const requestEmail = requestData.email ?? '';
const requestEmail = lodashGet(requestData, 'email', '');

// If we haven't passed an email in the request data, set it to the current user's email
if (email && _.isEmpty(requestEmail)) {
Expand Down
6 changes: 4 additions & 2 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ function createIOUTransaction(params) {
.then((data) => {
getIOUReportsForNewTransaction([data]);
Navigation.navigate(ROUTES.getReportRoute(data.chatReportID));
})?.catch((error) => {
})
.catch((error) => {
Onyx.merge(ONYXKEYS.IOU, {
loading: false,
creatingIOUTransaction: false,
Expand Down Expand Up @@ -134,7 +135,8 @@ function createIOUSplit(params) {
}
getIOUReportsForNewTransaction(reportParams);
Navigation.navigate(ROUTES.getReportRoute(chatReportID));
})?.catch((error) => {
})
.catch((error) => {
Onyx.merge(ONYXKEYS.IOU, {
loading: false,
creatingIOUTransaction: false,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/fileDownload/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default function fileDownload(url, fileName) {
link.style.display = 'none';
link.setAttribute(
'download',
fileName ?? getAttachmentName(url), // generating the file name
fileName || getAttachmentName(url), // generating the file name
);

// Append to html link element page
Expand Down
26 changes: 14 additions & 12 deletions src/libs/fileDownload/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function handleDownload(url, fileName) {
// android files will download to Download directory
// ios files will download to documents directory
const path = getPlatform() === 'android' ? dirs.DownloadDir : dirs.DocumentDir;
const attachmentName = fileName ?? getAttachmentName(url);
const attachmentName = fileName || getAttachmentName(url);

// fetching the attachment
const fetchedAttachment = RNFetchBlob.config({
Expand All @@ -75,18 +75,20 @@ function handleDownload(url, fileName) {

// resolving the fetched attachment
fetchedAttachment.then((attachment) => {
if (attachment?.info()) {
showAlert({
title: 'Downloaded!',
message: 'Attachment successfully downloaded',
options: [
{
text: 'OK',
style: 'cancel',
},
],
});
if (!attachment || !attachment.info()) {
return;
}

showAlert({
title: 'Downloaded!',
message: 'Attachment successfully downloaded',
options: [
{
text: 'OK',
style: 'cancel',
},
],
});
}).catch(() => {
showAlert({
title: 'Attachment Error',
Expand Down
3 changes: 2 additions & 1 deletion src/pages/NewChatPage.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashGet from 'lodash/get';
import _ from 'underscore';
import React, {Component} from 'react';
import {View} from 'react-native';
Expand Down Expand Up @@ -263,7 +264,7 @@ class NewChatPage extends Component {
forceTextUnreadStyle
/>
{!this.props.isGroupChat && <KeyboardSpacer />}
{this.props.isGroupChat && this.state.selectedOptions?.length > 0 && (
{this.props.isGroupChat && lodashGet(this.state, 'selectedOptions', []).length > 0 && (
<FixedFooter>
<Button
success
Expand Down
2 changes: 1 addition & 1 deletion src/pages/RequestCallPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ class RequestCallPage extends Component {
firstName,
firstNameError: '',
lastName,
phoneNumber: this.getPhoneNumber(props.user.loginList) || '',
lastNameError: '',
phoneNumber: this.getPhoneNumber(props.user.loginList) ?? '',
phoneNumberError: '',
isLoading: false,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {skinTones} from '../../../../../assets/emojis';
* @param {Number} skinToneIndex
* @returns {String}
*/
const getSkinToneEmojiFromIndex = skinToneIndex => skinTones.find(
emoji => emoji.skinTone === skinToneIndex,
) ?? skinTones[0];
function getSkinToneEmojiFromIndex(skinToneIndex) {
return skinTones.find(emoji => emoji.skinTone === skinToneIndex) || skinTones[0];
}

export default getSkinToneEmojiFromIndex;
4 changes: 2 additions & 2 deletions src/pages/iou/IOUModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ class IOUModal extends Component {
alternateText: Str.isSMSLogin(personalDetails.login) ? Str.removeSMSDomain(personalDetails.login) : personalDetails.login,
icons: [personalDetails.avatar],
keyForList: personalDetails.login,
payPalMeAddress: personalDetails.payPalMeAddress ?? '',
phoneNumber: personalDetails.phoneNumber ?? '',
payPalMeAddress: lodashGet(personalDetails, 'payPalMeAddress', ''),
phoneNumber: lodashGet(personalDetails, 'phoneNumber', ''),
}));

this.state = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashGet from 'lodash/get';
import React, {Component} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -230,7 +231,7 @@ class IOUParticipantsSplit extends Component {
forceTextUnreadStyle
/>
</View>
{this.props.participants?.length > 0 && (
{lodashGet(this.props, 'participants', []).length > 0 && (
<FixedFooter>
{maxParticipantsReached && (
<Text style={[styles.textLabelSupporting, styles.textAlignCenter, styles.mt1, styles.mb3]}>
Expand Down
3 changes: 2 additions & 1 deletion src/pages/settings/PreferencesPage.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashGet from 'lodash/get';
import React from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand Down Expand Up @@ -77,7 +78,7 @@ const PreferencesPage = ({
</View>
<View style={[styles.flex1, styles.alignItemsEnd]}>
<Switch
isOn={user.expensifyNewsStatus ?? true}
isOn={lodashGet(user, 'expensifyNewsStatus', true)}
onToggle={setExpensifyNewsStatus}
/>
</View>
Expand Down
5 changes: 3 additions & 2 deletions src/pages/settings/Profile/ProfilePage.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashGet from 'lodash/get';
import React, {Component} from 'react';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -97,8 +98,8 @@ class ProfilePage extends Component {
lastNameError: '',
pronouns: currentUserPronouns,
selfSelectedPronouns: initialSelfSelectedPronouns,
selectedTimezone: timezone.selected || CONST.DEFAULT_TIME_ZONE.selected,
isAutomaticTimezone: timezone.automatic ?? CONST.DEFAULT_TIME_ZONE.automatic,
selectedTimezone: lodashGet(timezone, 'selected', CONST.DEFAULT_TIME_ZONE.selected),
isAutomaticTimezone: lodashGet(timezone, 'automatic', CONST.DEFAULT_TIME_ZONE.automatic),
logins: this.getLogins(props.user.loginList),
};

Expand Down