Skip to content

Commit

Permalink
Merge pull request #40346 from bernhardoj/chore/39121-remove-underscore
Browse files Browse the repository at this point in the history
Remove underscore usage
  • Loading branch information
mountiny authored Apr 22, 2024
2 parents ad68a46 + 8ce0836 commit 42c494e
Show file tree
Hide file tree
Showing 25 changed files with 55 additions and 1,145 deletions.
8 changes: 1 addition & 7 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,7 @@ module.exports = {
'no-restricted-imports': [
'error',
{
paths: [
...restrictedImportPaths,
{
name: 'underscore',
message: 'Please use the corresponding method from lodash instead',
},
],
paths: restrictedImportPaths,
patterns: restrictedImportPatterns,
},
],
Expand Down
39 changes: 5 additions & 34 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ Using arrow functions is the preferred way to write an anonymous function such a

```javascript
// Bad
_.map(someArray, function (item) {...});
someArray.map(function (item) {...});

// Good
_.map(someArray, (item) => {...});
someArray.map((item) => {...});
```

Empty functions (noop) should be declare as arrow functions with no whitespace inside. Avoid _.noop()
Expand Down Expand Up @@ -112,38 +112,9 @@ if (someCondition) {
}
```

## 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)`.
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.

```javascript
// Bad
Expand Down Expand Up @@ -448,7 +419,7 @@ const propTypes = {
### Important Note:
In React Native, one **must not** attempt to falsey-check a string for an inline ternary. Even if it's in curly braces, React Native will try to render it as a `<Text>` node and most likely throw an error about trying to render text outside of a `<Text>` component. Use `_.isEmpty()` instead.
In React Native, one **must not** attempt to falsey-check a string for an inline ternary. Even if it's in curly braces, React Native will try to render it as a `<Text>` node and most likely throw an error about trying to render text outside of a `<Text>` component. Use `!!` instead.
```javascript
// Bad! This will cause a breaking an error on native platforms
Expand All @@ -467,7 +438,7 @@ In React Native, one **must not** attempt to falsey-check a string for an inline
{
return (
<View>
{!_.isEmpty(props.title)
{!!props.title
? <View style={styles.title}>{props.title}</View>
: null}
<View style={styles.body}>This is the body</View>
Expand Down
9 changes: 1 addition & 8 deletions package-lock.json

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

4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,7 @@
"react-webcam": "^7.1.1",
"react-window": "^1.8.9",
"semver": "^7.5.2",
"shim-keyboard-event-key": "^1.0.3",
"underscore": "^1.13.1"
"shim-keyboard-event-key": "^1.0.3"
},
"devDependencies": {
"@actions/core": "1.10.0",
Expand Down Expand Up @@ -233,7 +232,6 @@
"@types/react-test-renderer": "^18.0.0",
"@types/semver": "^7.5.4",
"@types/setimmediate": "^1.0.2",
"@types/underscore": "^1.11.5",
"@types/webpack": "^5.28.5",
"@types/webpack-bundle-analyzer": "^4.7.0",
"@typescript-eslint/eslint-plugin": "^6.13.2",
Expand Down
5 changes: 0 additions & 5 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,6 @@ const ROUTES = {
route: 'r/:reportID/avatar',
getRoute: (reportID: string) => `r/${reportID}/avatar` as const,
},
EDIT_REQUEST: {
route: 'r/:threadReportID/edit/:field/:tagIndex?',
getRoute: (threadReportID: string, field: ValueOf<typeof CONST.EDIT_REQUEST_FIELD>, tagIndex?: number) =>
`r/${threadReportID}/edit/${field as string}${typeof tagIndex === 'number' ? `/${tagIndex}` : ''}` as const,
},
EDIT_CURRENCY_REQUEST: {
route: 'r/:threadReportID/edit/currency',
getRoute: (threadReportID: string, currency: string, backTo: string) => `r/${threadReportID}/edit/currency?currency=${currency}&backTo=${backTo}` as const,
Expand Down
1 change: 0 additions & 1 deletion src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ const SCREENS = {
},

EDIT_REQUEST: {
ROOT: 'EditRequest_Root',
CURRENCY: 'EditRequest_Currency',
REPORT_FIELD: 'EditRequest_ReportField',
},
Expand Down
2 changes: 0 additions & 2 deletions src/components/FloatingActionButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,10 @@ type AdapterProps = {

const adapter = createAnimatedPropAdapter(
(props: AdapterProps) => {
// eslint-disable-next-line rulesdir/prefer-underscore-method
if (Object.keys(props).includes('fill')) {
// eslint-disable-next-line no-param-reassign
props.fill = {type: 0, payload: processColor(props.fill)};
}
// eslint-disable-next-line rulesdir/prefer-underscore-method
if (Object.keys(props).includes('stroke')) {
// eslint-disable-next-line no-param-reassign
props.stroke = {type: 0, payload: processColor(props.stroke)};
Expand Down
89 changes: 0 additions & 89 deletions src/components/Modal/modalPropTypes.js

This file was deleted.

39 changes: 22 additions & 17 deletions src/components/OptionsSelector/BaseOptionsSelector.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import lodashDebounce from 'lodash/debounce';
import lodashFind from 'lodash/find';
import lodashFindIndex from 'lodash/findIndex';
import lodashGet from 'lodash/get';
import lodashIsEqual from 'lodash/isEqual';
import lodashMap from 'lodash/map';
import lodashValues from 'lodash/values';
import PropTypes from 'prop-types';
import React, {Component} from 'react';
import {View} from 'react-native';
import _ from 'underscore';
import ArrowKeyFocusManager from '@components/ArrowKeyFocusManager';
import Button from '@components/Button';
import FixedFooter from '@components/FixedFooter';
Expand Down Expand Up @@ -77,9 +82,9 @@ class BaseOptionsSelector extends Component {
this.calculateAllVisibleOptionsCount = this.calculateAllVisibleOptionsCount.bind(this);
this.handleFocusIn = this.handleFocusIn.bind(this);
this.handleFocusOut = this.handleFocusOut.bind(this);
this.debouncedUpdateSearchValue = _.debounce(this.updateSearchValue, CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
this.debouncedUpdateSearchValue = lodashDebounce(this.updateSearchValue, CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
this.relatedTarget = null;
this.accessibilityRoles = _.values(CONST.ROLE);
this.accessibilityRoles = lodashValues(CONST.ROLE);
this.isWebOrDesktop = [CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform());

const allOptions = this.flattenSections();
Expand Down Expand Up @@ -155,7 +160,7 @@ class BaseOptionsSelector extends Component {
this.focusedOption = this.state.allOptions[this.state.focusedIndex];
}

if (_.isEqual(this.props.sections, prevProps.sections)) {
if (lodashIsEqual(this.props.sections, prevProps.sections)) {
return;
}

Expand All @@ -171,14 +176,14 @@ class BaseOptionsSelector extends Component {
}
const newFocusedIndex = this.props.selectedOptions.length;
const isNewFocusedIndex = newFocusedIndex !== this.state.focusedIndex;
const prevFocusedOption = _.find(newOptions, (option) => this.focusedOption && option.keyForList === this.focusedOption.keyForList);
const prevFocusedOptionIndex = prevFocusedOption ? _.findIndex(newOptions, (option) => this.focusedOption && option.keyForList === this.focusedOption.keyForList) : undefined;
const prevFocusedOption = lodashFind(newOptions, (option) => this.focusedOption && option.keyForList === this.focusedOption.keyForList);
const prevFocusedOptionIndex = prevFocusedOption ? lodashFindIndex(newOptions, (option) => this.focusedOption && option.keyForList === this.focusedOption.keyForList) : undefined;
// eslint-disable-next-line react/no-did-update-set-state
this.setState(
{
sections: newSections,
allOptions: newOptions,
focusedIndex: prevFocusedOptionIndex || (_.isNumber(this.props.focusedIndex) ? this.props.focusedIndex : newFocusedIndex),
focusedIndex: prevFocusedOptionIndex || (typeof this.props.focusedIndex === 'number' ? this.props.focusedIndex : newFocusedIndex),
},
() => {
// If we just toggled an option on a multi-selection page or cleared the search input, scroll to top
Expand Down Expand Up @@ -230,11 +235,11 @@ class BaseOptionsSelector extends Component {
} else {
defaultIndex = this.props.selectedOptions.length;
}
if (_.isUndefined(this.props.initiallyFocusedOptionKey)) {
if (this.props.initiallyFocusedOptionKey === undefined) {
return defaultIndex;
}

const indexOfInitiallyFocusedOption = _.findIndex(allOptions, (option) => option.keyForList === this.props.initiallyFocusedOptionKey);
const indexOfInitiallyFocusedOption = lodashFindIndex(allOptions, (option) => option.keyForList === this.props.initiallyFocusedOptionKey);

return indexOfInitiallyFocusedOption;
}
Expand All @@ -245,8 +250,8 @@ class BaseOptionsSelector extends Component {
* @returns {Objects[]}
*/
sliceSections() {
return _.map(this.props.sections, (section) => {
if (_.isEmpty(section.data)) {
return lodashMap(this.props.sections, (section) => {
if (section.data.length === 0) {
return section;
}

Expand All @@ -266,7 +271,7 @@ class BaseOptionsSelector extends Component {
calculateAllVisibleOptionsCount() {
let count = 0;

_.forEach(this.state.sections, (section) => {
this.state.sections.forEach((section) => {
count += lodashGet(section, 'data.length', 0);
});

Expand Down Expand Up @@ -347,7 +352,7 @@ class BaseOptionsSelector extends Component {

selectFocusedOption(e) {
const focusedItemKey = lodashGet(e, ['target', 'attributes', 'id', 'value']);
const focusedOption = focusedItemKey ? _.find(this.state.allOptions, (option) => option.keyForList === focusedItemKey) : this.state.allOptions[this.state.focusedIndex];
const focusedOption = focusedItemKey ? lodashFind(this.state.allOptions, (option) => option.keyForList === focusedItemKey) : this.state.allOptions[this.state.focusedIndex];

if (!focusedOption || !this.props.isFocused) {
return;
Expand Down Expand Up @@ -393,8 +398,8 @@ class BaseOptionsSelector extends Component {
const allOptions = [];
this.disabledOptionsIndexes = [];
let index = 0;
_.each(this.props.sections, (section, sectionIndex) => {
_.each(section.data, (option, optionIndex) => {
this.props.sections.forEach((section, sectionIndex) => {
section.data.forEach((option, optionIndex) => {
allOptions.push({
...option,
sectionIndex,
Expand Down Expand Up @@ -496,8 +501,8 @@ class BaseOptionsSelector extends Component {
render() {
const shouldShowShowMoreButton = this.state.allOptions.length > CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * this.state.paginationPage;
const shouldShowFooter =
!this.props.isReadOnly && (this.props.shouldShowConfirmButton || this.props.footerContent) && !(this.props.canSelectMultipleOptions && _.isEmpty(this.props.selectedOptions));
const defaultConfirmButtonText = _.isUndefined(this.props.confirmButtonText) ? this.props.translate('common.confirm') : this.props.confirmButtonText;
!this.props.isReadOnly && (this.props.shouldShowConfirmButton || this.props.footerContent) && !(this.props.canSelectMultipleOptions && this.props.selectedOptions.length === 0);
const defaultConfirmButtonText = this.props.confirmButtonText === undefined ? this.props.translate('common.confirm') : this.props.confirmButtonText;
const shouldShowDefaultConfirmButton = !this.props.footerContent && defaultConfirmButtonText;
const safeAreaPaddingBottomStyle = shouldShowFooter ? undefined : this.props.safeAreaPaddingBottomStyle;
const listContainerStyles = this.props.listContainerStyles || [this.props.themeStyles.flex1];
Expand Down
Loading

0 comments on commit 42c494e

Please sign in to comment.