Skip to content
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
46 changes: 0 additions & 46 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
- [Forwarding refs](#forwarding-refs)
- [Hooks and HOCs](#hooks-and-hocs)
- [Stateless components vs Pure Components vs Class based components vs Render Props](#stateless-components-vs-pure-components-vs-class-based-components-vs-render-props---when-to-use-what)
- [Composition](#composition)
- [Use Refs Appropriately](#use-refs-appropriately)
- [Are we allowed to use [insert brand new React feature]?](#are-we-allowed-to-use-insert-brand-new-react-feature-why-or-why-not)
- [React Hooks: Frequently Asked Questions](#react-hooks-frequently-asked-questions)
Expand Down Expand Up @@ -1075,51 +1074,6 @@ Class components are DEPRECATED. Use function components and React hooks.

[https://react.dev/reference/react/Component#migrating-a-component-with-lifecycle-methods-from-a-class-to-a-function](https://react.dev/reference/react/Component#migrating-a-component-with-lifecycle-methods-from-a-class-to-a-function)

### Composition

Avoid the usage of `compose` function to compose HOCs in TypeScript files. Use nesting instead.

> Why? `compose` function doesn't work well with TypeScript when dealing with several HOCs being used in a component, many times resulting in wrong types and errors. Instead, nesting can be used to allow a seamless use of multiple HOCs and result in a correct return type of the compoment. Also, you can use [hooks instead of HOCs](#hooks-instead-of-hocs) whenever possible to minimize or even remove the need of HOCs in the component.

From React's documentation -
>Props and composition give you all the flexibility you need to customize a component’s look and behavior in an explicit and safe way. Remember that components may accept arbitrary props, including primitive values, React elements, or functions.
>If you want to reuse non-UI functionality between components, we suggest extracting it into a separate JavaScript module. The components may import it and use that function, object, or a class, without extending it.

```ts
// BAD
export default compose(
withCurrentUserPersonalDetails,
withReportOrNotFound(),
withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
}),
)(Component);

// GOOD
export default withCurrentUserPersonalDetails(
withReportOrNotFound()(
withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Component),
),
);

// GOOD - alternative to HOC nesting
const ComponentWithOnyx = withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Component);
const ComponentWithReportOrNotFound = withReportOrNotFound()(ComponentWithOnyx);
export default withCurrentUserPersonalDetails(ComponentWithReportOrNotFound);
```

**Note:** If you find that none of these approaches work for you, please ask an Expensify engineer for guidance via Slack or GitHub.

### Use Refs Appropriately

React's documentation explains refs in [detail](https://reactjs.org/docs/refs-and-the-dom.html). It's important to understand when to use them and how to use them to avoid bugs and hard to maintain code.
Expand Down
12 changes: 5 additions & 7 deletions src/components/LocaleContextProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {createContext, useMemo} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import compose from '@libs/compose';
import DateUtils from '@libs/DateUtils';
import * as LocaleDigitUtils from '@libs/LocaleDigitUtils';
import * as LocalePhoneNumber from '@libs/LocalePhoneNumber';
Expand Down Expand Up @@ -125,18 +124,17 @@ function LocaleContextProvider({preferredLocale, currentUserPersonalDetails = {}
return <LocaleContext.Provider value={contextValue}>{children}</LocaleContext.Provider>;
}

const Provider = compose(
const Provider = withCurrentUserPersonalDetails(
withOnyx<LocaleContextProviderProps, LocaleContextProviderOnyxProps>({
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
selector: (preferredLocale) => preferredLocale,
},
}),
withCurrentUserPersonalDetails,
)(LocaleContextProvider);
})(LocaleContextProvider),
);

Provider.displayName = 'withOnyx(LocaleContextProvider)';

export {Provider as LocaleContextProvider, LocaleContext};
export {LocaleContext, Provider as LocaleContextProvider};

export type {LocaleContextProps, Locale};
export type {Locale, LocaleContextProps};
14 changes: 5 additions & 9 deletions src/components/MapView/MapView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {PressableWithoutFeedback} from '@components/Pressable';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import * as UserLocation from '@libs/actions/UserLocation';
import compose from '@libs/compose';
import getCurrentPosition from '@libs/getCurrentPosition';
import type {GeolocationErrorCallback} from '@libs/getCurrentPosition/getCurrentPosition.types';
import {GeolocationErrorCode} from '@libs/getCurrentPosition/getCurrentPosition.types';
Expand Down Expand Up @@ -265,11 +264,8 @@ const MapView = forwardRef<MapViewHandle, ComponentProps>(
},
);

export default compose(
withOnyx<ComponentProps, MapViewOnyxProps>({
userLocation: {
key: ONYXKEYS.USER_LOCATION,
},
}),
memo,
)(MapView);
export default withOnyx<ComponentProps, MapViewOnyxProps>({
userLocation: {
key: ONYXKEYS.USER_LOCATION,
},
})(memo(MapView));
8 changes: 3 additions & 5 deletions src/components/TestToolMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {withOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ApiUtils from '@libs/ApiUtils';
import compose from '@libs/compose';
import * as Network from '@userActions/Network';
import * as Session from '@userActions/Session';
import * as User from '@userActions/User';
Expand Down Expand Up @@ -95,11 +94,10 @@ function TestToolMenu({user = USER_DEFAULT, network}: TestToolMenuProps) {

TestToolMenu.displayName = 'TestToolMenu';

export default compose(
export default withNetwork()(
withOnyx<TestToolMenuProps, TestToolMenuOnyxProps>({
user: {
key: ONYXKEYS.USER,
},
}),
withNetwork(),
)(TestToolMenu);
})(TestToolMenu),
);
71 changes: 0 additions & 71 deletions src/libs/compose.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {withOnyx} from 'react-native-onyx';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import withWindowDimensions from '@components/withWindowDimensions';
import type {WindowDimensionsProps} from '@components/withWindowDimensions/types';
import compose from '@libs/compose';
import getComponentDisplayName from '@libs/getComponentDisplayName';
import type {FlagCommentNavigatorParamList, SplitDetailsNavigatorParamList} from '@libs/Navigation/types';
import * as ReportUtils from '@libs/ReportUtils';
Expand Down Expand Up @@ -103,7 +102,7 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps

WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`;

return compose(
Comment thread
abzokhattab marked this conversation as resolved.
return withWindowDimensions(
withOnyx<TProps & RefAttributes<TRef>, OnyxProps>({
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`,
Expand Down Expand Up @@ -138,9 +137,8 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps
},
canEvict: false,
},
}),
withWindowDimensions,
)(React.forwardRef(WithReportOrNotFound));
})(React.forwardRef(WithReportOrNotFound)),
);
}

export type {WithReportAndReportActionOrNotFoundProps};
Original file line number Diff line number Diff line change
@@ -1,36 +1,31 @@
import {Str} from 'expensify-common';
import React, {useEffect} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import {withNetwork} from '@components/OnyxProvider';
import ScrollView from '@components/ScrollView';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import {getUnitTranslationKey} from '@libs/WorkspacesSettingsUtils';
import withPolicy from '@pages/workspace/withPolicy';
import type {WithPolicyProps} from '@pages/workspace/withPolicy';
import withPolicy from '@pages/workspace/withPolicy';
import WorkspacePageWithSections from '@pages/workspace/WorkspacePageWithSections';
import * as BankAccounts from '@userActions/BankAccounts';
import * as Policy from '@userActions/Policy/Policy';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Network, ReimbursementAccount, WorkspaceRateAndUnit} from '@src/types/onyx';
import type {ReimbursementAccount, WorkspaceRateAndUnit} from '@src/types/onyx';
import type {Unit} from '@src/types/onyx/Policy';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

type WorkspaceRateAndUnitPageBaseProps = WithPolicyProps & {
// eslint-disable-next-line react/no-unused-prop-types
network: OnyxEntry<Network>;
};
type WorkspaceRateAndUnitPageBaseProps = WithPolicyProps;

type WorkspaceRateAndUnitOnyxProps = {
workspaceRateAndUnit: OnyxEntry<WorkspaceRateAndUnit>;
Expand Down Expand Up @@ -150,7 +145,7 @@ function WorkspaceRateAndUnitPage(props: WorkspaceRateAndUnitPageProps) {

WorkspaceRateAndUnitPage.displayName = 'WorkspaceRateAndUnitPage';

export default compose(
export default withPolicy(
withOnyx<WorkspaceRateAndUnitPageProps, WorkspaceRateAndUnitOnyxProps>({
// @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM
reimbursementAccount: {
Expand All @@ -159,7 +154,5 @@ export default compose(
workspaceRateAndUnit: {
key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT,
},
}),
withPolicy,
withNetwork(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was this not needed in the page?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes its not used anywhere as specified in the props types:

type WorkspaceRateAndUnitPageBaseProps = WithPolicyProps & {
// eslint-disable-next-line react/no-unused-prop-types
network: OnyxEntry<Network>;
};

also i forgot to remove this type, i just removed it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually, as i mentioned here #42069 (comment) the whole ( InitialPage, RatePage,UnitPage ) pages are not used anywhere so i am not sure if we should remove these components or not and if so should we do it in the same PR or in a new one.

)(WorkspaceRateAndUnitPage);
})(WorkspaceRateAndUnitPage),
);
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import React, {useCallback, useEffect, useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import AmountForm from '@components/AmountForm';
import FormProvider from '@components/Form/FormProvider';
import InputWrapperWithRef from '@components/Form/InputWrapper';
import type {FormOnyxValues} from '@components/Form/types';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import Navigation from '@libs/Navigation/Navigation';
import {validateRateValue} from '@libs/PolicyDistanceRatesUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import withPolicy from '@pages/workspace/withPolicy';
import type {WithPolicyProps} from '@pages/workspace/withPolicy';
import withPolicy from '@pages/workspace/withPolicy';
import WorkspacePageWithSections from '@pages/workspace/WorkspacePageWithSections';
import * as Policy from '@userActions/Policy/Policy';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -99,11 +98,10 @@ function WorkspaceRatePage(props: WorkspaceRatePageProps) {

WorkspaceRatePage.displayName = 'WorkspaceRatePage';

export default compose(
export default withPolicy(
withOnyx<WorkspaceRatePageProps, WorkspaceRateAndUnitOnyxProps>({
workspaceRateAndUnit: {
key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT,
},
}),
withPolicy,
)(WorkspaceRatePage);
})(WorkspaceRatePage),
);
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import React, {useEffect, useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import Text from '@components/Text';
import type {UnitItemType} from '@components/UnitPicker';
import UnitPicker from '@components/UnitPicker';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import withPolicy from '@pages/workspace/withPolicy';
import type {WithPolicyProps} from '@pages/workspace/withPolicy';
import withPolicy from '@pages/workspace/withPolicy';
import WorkspacePageWithSections from '@pages/workspace/WorkspacePageWithSections';
import * as Policy from '@userActions/Policy/Policy';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -70,12 +69,10 @@ function WorkspaceUnitPage(props: WorkspaceUnitPageProps) {
}

WorkspaceUnitPage.displayName = 'WorkspaceUnitPage';

export default compose(
export default withPolicy(
withOnyx<WorkspaceUnitPageProps, WorkspaceRateAndUnitOnyxProps>({
workspaceRateAndUnit: {
key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT,
},
}),
withPolicy,
)(WorkspaceUnitPage);
})(WorkspaceUnitPage),
);
Loading