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] Update all array methods to use underscore #5993

Merged
merged 10 commits into from
Oct 25, 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
29 changes: 29 additions & 0 deletions STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,35 @@ Empty functions (noop) should be declare as arrow functions with no whitespace i
}
```

## Object / Array Methods

We have standardized on using [underscore.js](https://underscorejs.org/) methods for objects and collections instead of the native [Array instance methods](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#instance_methods). This is mostly to maintain consistency, but there are some type safety features and conveniences that underscore methods provide us e.g. the ability to iterate over an object and the lack of a `TypeError` thrown if a variable is `undefined`.

```javascript
// Bad
myArray.forEach(item => doSomething(item));
// Good
_.each(myArray, item => doSomething(item));

// Bad
const myArray = Object.keys(someObject).map(key => doSomething(someObject[key]));
// Good
const myArray = _.map(someObject, (value, key) => doSomething(value));

// Bad
myCollection.includes('item');
// Good
_.contains(myCollection, 'item');

// Bad
const modifiedArray = someArray.filter(filterFunc).map(mapFunc);
// Good
const modifiedArray = _.chain(someArray)
.filter(filterFunc)
.map(mapFunc)
.value();
```

## Accessing Object Properties and Default Values

Use `lodashGet()` 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)`.
Expand Down
4 changes: 3 additions & 1 deletion __mocks__/console.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import _ from 'underscore';

function format(entry) {
if (typeof entry === 'object') {
try {
Expand All @@ -10,7 +12,7 @@ function format(entry) {
}

function log(...msgs) {
process.stdout.write(`${msgs.map(format).join(' ')}\n`);
process.stdout.write(`${_.map(msgs, format).join(' ')}\n`);
}

module.exports = {
Expand Down
12 changes: 7 additions & 5 deletions __mocks__/react-native-permissions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const {PERMISSIONS} = require('react-native-permissions/dist/commonjs/permissions');
const {RESULTS} = require('react-native-permissions/dist/commonjs/results');
const _ = require('underscore');

export {PERMISSIONS, RESULTS};

Expand Down Expand Up @@ -30,20 +31,21 @@ const checkNotifications = jest.fn(() => ({

const requestNotifications = jest.fn(options => ({
status: RESULTS.GRANTED,
settings: options
.filter(option => notificationOptions.includes(option))
settings: _.chain(options)
.filter(option => _.contains(notificationOptions, option))
.reduce((acc, option) => ({...acc, [option]: true}), {
lockScreen: true,
notificationCenter: true,
}),
})
.value(),
}));

const checkMultiple = jest.fn(permissions => permissions.reduce((acc, permission) => ({
const checkMultiple = jest.fn(permissions => _.reduce(permissions, (acc, permission) => ({
...acc,
[permission]: RESULTS.GRANTED,
})));

const requestMultiple = jest.fn(permissions => permissions.reduce((acc, permission) => ({
const requestMultiple = jest.fn(permissions => _.reduce(permissions, (acc, permission) => ({
...acc,
[permission]: RESULTS.GRANTED,
})));
Expand Down
3 changes: 2 additions & 1 deletion src/components/AttachmentPicker/index.native.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* The react native image/document pickers work for iOS/Android, but we want to wrap them both within AttachmentPicker
*/
import _ from 'underscore';
import React, {Component} from 'react';
import {Alert, Linking, View} from 'react-native';
import {launchImageLibrary} from 'react-native-image-picker';
Expand Down Expand Up @@ -284,7 +285,7 @@ class AttachmentPicker extends Component {
>
<View style={this.props.isSmallScreenWidth ? {} : styles.createMenuContainer}>
{
this.menuItemData.map(item => (
_.map(this.menuItemData, item => (
<MenuItem
key={item.textTranslationKey}
icon={item.icon}
Expand Down
3 changes: 2 additions & 1 deletion src/components/ComposeProviders.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React from 'react';
import PropTypes from 'prop-types';

Expand All @@ -11,7 +12,7 @@ const propTypes = {

const ComposeProviders = props => (
<>
{props.components.reduceRight((memo, Component) => (
{_.reduceRight(props.components, (memo, Component) => (
<Component>{memo}</Component>
), props.children)}
</>
Expand Down
3 changes: 2 additions & 1 deletion src/components/Icon/BankIcons.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import {CreditCard} from './Expensicons';
import AmericanExpress from '../../../assets/images/bankicons/american-express.svg';
import BankOfAmerica from '../../../assets/images/bankicons/bank-of-america.svg';
Expand Down Expand Up @@ -120,7 +121,7 @@ export default function getBankIcon(bankName, isCard) {
}

// For default Credit Card icon the icon size should not be set.
if (![CreditCard].includes(bankIcon.icon)) {
if (!_.contains([CreditCard], bankIcon.icon)) {
bankIcon.iconSize = variables.iconSizeExtraLarge;
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/InlineCodeBlock/WrappedText.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import Text from '../Text';
* @returns {Array<String[]>}
*/
function getTextMatrix(text) {
return text.split('\n').map(row => _.without(row.split(/(\s)/), ''));
return _.map(text.split('\n'), row => _.without(row.split(/(\s)/), ''));
}

const propTypes = {
Expand All @@ -39,12 +39,12 @@ const WrappedText = (props) => {
const textMatrix = getTextMatrix(props.children);
return (
<>
{textMatrix.map((rowText, rowIndex) => (
{_.map(textMatrix, (rowText, rowIndex) => (
<Fragment
// eslint-disable-next-line react/no-array-index-key
key={`${rowText}-${rowIndex}`}
>
{rowText.map((colText, colIndex) => (
{_.map(rowText, (colText, colIndex) => (

// Outer View is important to vertically center the Text
<View
Expand Down
3 changes: 2 additions & 1 deletion src/components/KeyboardSpacer/index.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* On iOS the keyboard covers the input fields on the bottom of the view. This component moves the view up with the
* keyboard allowing the user to see what they are typing.
*/
import _ from 'underscore';
import ReactNativeKeyboardSpacer from 'react-native-keyboard-spacer';
import React from 'react';
import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions';
Expand All @@ -20,7 +21,7 @@ const KeyboardSpacer = (props) => {
*/
function hasSafeAreas(windowWidth, windowHeight) {
const heightsIphonesWithNotches = [812, 896, 844, 926];
return (heightsIphonesWithNotches.includes(windowHeight) || heightsIphonesWithNotches.includes(windowWidth));
return _.contains(heightsIphonesWithNotches, windowHeight) || _.contains(heightsIphonesWithNotches, windowWidth);
}

return (
Expand Down
3 changes: 2 additions & 1 deletion src/components/LocalePicker.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React from 'react';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -56,7 +57,7 @@ const LocalePicker = ({
setLocale(locale);
}
}}
items={Object.values(localesToLanguages)}
items={_.values(localesToLanguages)}
size={size}
value={preferredLocale}
/>
Expand Down
3 changes: 2 additions & 1 deletion src/components/PopoverMenu/BasePopoverMenu.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React, {PureComponent} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -45,7 +46,7 @@ class BasePopoverMenu extends PureComponent {
</Text>
</View>
)}
{this.props.menuItems.map(item => (
{_.map(this.props.menuItems, item => (
<MenuItem
key={item.text}
icon={item.icon}
Expand Down
7 changes: 4 additions & 3 deletions src/components/VideoChatButtonAndMenu.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React, {Component} from 'react';
import {
View, Pressable, Dimensions, Linking,
Expand Down Expand Up @@ -36,7 +37,7 @@ class VideoChatButtonAndMenu extends Component {
this.toggleVideoChatMenu = this.toggleVideoChatMenu.bind(this);
this.measureVideoChatIconPosition = this.measureVideoChatIconPosition.bind(this);
this.videoChatIconWrapper = null;
this.menuItemData = [
this.menuItemData = _.map([
{
icon: ZoomIcon,
text: props.translate('videoChatButtonAndMenu.zoom'),
Expand All @@ -47,7 +48,7 @@ class VideoChatButtonAndMenu extends Component {
text: props.translate('videoChatButtonAndMenu.googleMeet'),
onPress: () => Linking.openURL(CONST.NEW_GOOGLE_MEET_MEETING_URL),
},
].map(item => ({
], item => ({
...item,
onPress: () => {
item.onPress();
Expand Down Expand Up @@ -127,7 +128,7 @@ class VideoChatButtonAndMenu extends Component {
animationIn="fadeInDown"
animationOut="fadeOutUp"
>
{this.menuItemData.map(({icon, text, onPress}) => (
{_.map(this.menuItemData, ({icon, text, onPress}) => (
<MenuItem
wrapperStyle={styles.mr3}
key={text}
Expand Down
3 changes: 2 additions & 1 deletion src/libs/Navigation/CustomActions.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import {CommonActions, StackActions, DrawerActions} from '@react-navigation/native';
import lodashGet from 'lodash/get';

Expand Down Expand Up @@ -73,7 +74,7 @@ function pushDrawerRoute(screenName, params, navigationRef) {
}

const screenRoute = {type: 'route', name: screenName};
const history = [...(state.history || [])].map(() => screenRoute);
const history = _.map([...(state.history || [])], () => screenRoute);
history.push(screenRoute);
return CommonActions.reset({
...state,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Web and desktop implementation only. Do not import for direct use. Use LocalNotification.
import _ from 'underscore';
import Str from 'expensify-common/lib/str';
import Onyx from 'react-native-onyx';
import focusApp from './focusApp';
Expand Down Expand Up @@ -107,10 +108,10 @@ export default {
*/
pushReportCommentNotification({reportAction, onClick}) {
const {person, message} = reportAction;
const plainTextPerson = Str.htmlDecode(person.map(f => f.text).join());
const plainTextPerson = Str.htmlDecode(_.map(person, f => f.text).join());

// Specifically target the comment part of the message
const plainTextMessage = Str.htmlDecode((message.find(f => f.type === 'COMMENT') || {}).text);
const plainTextMessage = Str.htmlDecode((_.find(message, f => f.type === 'COMMENT') || {}).text);

push({
title: `New message from ${plainTextPerson}`,
Expand Down
27 changes: 14 additions & 13 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,12 @@ function getSearchText(report, personalDetailList, isDefaultChatRoom) {
}
if (report) {
searchTerms.push(...report.reportName);
searchTerms.push(...report.reportName.split(',').map(name => name.trim()));
searchTerms.push(..._.map(report.reportName.split(','), name => name.trim()));

if (isDefaultChatRoom) {
const defaultRoomSubtitle = getDefaultRoomSubtitle(report, policies);
searchTerms.push(...defaultRoomSubtitle);
searchTerms.push(...defaultRoomSubtitle.split(',').map(name => name.trim()));
searchTerms.push(..._.map(defaultRoomSubtitle.split(','), name => name.trim()));
} else {
searchTerms.push(...report.participants);
}
Expand Down Expand Up @@ -222,8 +222,7 @@ function createOption(personalDetailList, report, draftComments, {
: getDefaultRoomSubtitle(report, policies);
} else {
text = hasMultipleParticipants
? personalDetailList
.map(({firstName, login}) => firstName || Str.removeSMSDomain(login))
? _.map(personalDetailList, ({firstName, login}) => firstName || Str.removeSMSDomain(login))
.join(', ')
: lodashGet(report, ['reportName'], personalDetail.displayName);
alternateText = (showChatPreviewLine && lastMessageText)
Expand Down Expand Up @@ -267,10 +266,12 @@ function createOption(personalDetailList, report, draftComments, {
* @returns {Boolean}
*/
function isSearchStringMatch(searchValue, searchText, participantNames = new Set(), isDefaultChatRoom = false) {
const searchWords = searchValue
.replace(/,/g, ' ')
.split(' ')
.map(word => word.trim());
const searchWords = _.map(
searchValue
.replace(/,/g, ' ')
.split(' '),
word => word.trim(),
);
return _.every(searchWords, (word) => {
const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i');
const valueToSearch = searchText && searchText.replace(new RegExp(/&nbsp;/g), '');
Expand Down Expand Up @@ -593,7 +594,7 @@ function getIOUConfirmationOptionsFromMyPersonalDetail(myPersonalDetail, amountT
function getIOUConfirmationOptionsFromParticipants(
participants, amountText,
) {
return participants.map(participant => ({
return _.map(participants, participant => ({
...participant, descriptiveText: amountText,
}));
}
Expand Down Expand Up @@ -711,7 +712,7 @@ function getHeaderMessage(hasSelectableOptions, hasUserToInvite, searchValue, ma
* @returns {Array}
*/
function getCurrencyListForSections(currencyOptions, searchValue) {
const filteredOptions = currencyOptions.filter(currencyOption => (
const filteredOptions = _.filter(currencyOptions, currencyOption => (
isSearchStringMatch(searchValue, currencyOption.searchText)));

return {
Expand All @@ -732,13 +733,13 @@ function getReportIcons(report, personalDetails) {
if (isDefaultRoom(report)) {
return [''];
}
return _.map(report.participants, dmParticipant => ({
const sortedParticipants = _.map(report.participants, dmParticipant => ({
firstName: lodashGet(personalDetails, [dmParticipant, 'firstName'], ''),
avatar: lodashGet(personalDetails, [dmParticipant, 'avatarThumbnail'], '')
|| getDefaultAvatar(dmParticipant),
}))
.sort((first, second) => first.firstName - second.firstName)
.map(item => item.avatar);
.sort((first, second) => first.firstName - second.firstName);
return _.map(sortedParticipants, item => item.avatar);
}

export {
Expand Down
7 changes: 4 additions & 3 deletions src/libs/Performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,15 @@ if (canCapturePerformanceMetrics()) {
* Outputs performance stats. We alert these so that they are easy to access in release builds.
*/
Performance.printPerformanceMetrics = () => {
const stats = [
const stats = _.chain([
...rnPerformance.getEntriesByName('nativeLaunch'),
...rnPerformance.getEntriesByName('runJsBundle'),
...rnPerformance.getEntriesByName('jsBundleDownload'),
...rnPerformance.getEntriesByName('TTI'),
]
])
.filter(entry => entry.duration > 0)
.map(entry => `\u2022 ${entry.name}: ${entry.duration.toFixed(1)}ms`);
.map(entry => `\u2022 ${entry.name}: ${entry.duration.toFixed(1)}ms`)
.value();

Alert.alert('Performance', stats.join('\n'));
};
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Pusher/pusher.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ function bindEventToChannel(channel, eventName, eventCallback = () => {}, isChun

// Only call the event callback if we've received the last packet and we don't have any holes in the complete
// packet.
if (chunkedEvent.receivedFinal && chunkedEvent.chunks.length === Object.keys(chunkedEvent.chunks).length) {
if (chunkedEvent.receivedFinal && chunkedEvent.chunks.length === _.keys(chunkedEvent.chunks).length) {
eventCallback(JSON.parse(chunkedEvent.chunks.join('')));
try {
eventCallback(JSON.parse(chunkedEvent.chunks.join('')));
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/BankAccounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ function activateWallet(currentStep, parameters) {
CONST.WALLET.ERROR.UNABLE_TO_VERIFY,
];

if (errorTitles.includes(response.title)) {
if (_.contains(errorTitles, response.title)) {
setAdditionalDetailsStep(false, null, response.message);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ function createIOUSplit(params) {

let chatReportID;
API.CreateChatReport({
emailList: params.splits.map(participant => participant.email).join(','),
emailList: _.map(params.splits, participant => participant.email).join(','),
})
.then((data) => {
chatReportID = data.reportID;
Expand Down
Loading