From 4400aca0b2948f8a2945e16335975152851df5b6 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Fri, 10 May 2024 12:13:24 +0200 Subject: [PATCH 1/6] Removing compose.ts --- src/components/LocaleContextProvider.tsx | 12 +++---- src/components/MapView/MapView.tsx | 8 ++--- src/components/TestToolMenu.tsx | 8 ++--- .../withReportAndReportActionOrNotFound.tsx | 8 ++--- .../WorkspaceRateAndUnitPage/InitialPage.tsx | 31 +++++++++---------- .../WorkspaceRateAndUnitPage/RatePage.tsx | 12 +++---- .../WorkspaceRateAndUnitPage/UnitPage.tsx | 13 +++----- .../withPolicyAndFullscreenLoading.tsx | 8 ++--- 8 files changed, 42 insertions(+), 58 deletions(-) diff --git a/src/components/LocaleContextProvider.tsx b/src/components/LocaleContextProvider.tsx index eb7d9324d2ab..e0e30d14d2a2 100644 --- a/src/components/LocaleContextProvider.tsx +++ b/src/components/LocaleContextProvider.tsx @@ -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'; @@ -125,18 +124,17 @@ function LocaleContextProvider({preferredLocale, currentUserPersonalDetails = {} return {children}; } -const Provider = compose( +const Provider = withCurrentUserPersonalDetails( withOnyx({ 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}; diff --git a/src/components/MapView/MapView.tsx b/src/components/MapView/MapView.tsx index b1021350435c..05a7cd221c1e 100644 --- a/src/components/MapView/MapView.tsx +++ b/src/components/MapView/MapView.tsx @@ -6,7 +6,6 @@ import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import useThemeStyles from '@hooks/useThemeStyles'; import setUserLocation from '@libs/actions/UserLocation'; -import compose from '@libs/compose'; import getCurrentPosition from '@libs/getCurrentPosition'; import CONST from '@src/CONST'; import useLocalize from '@src/hooks/useLocalize'; @@ -196,11 +195,10 @@ const MapView = forwardRef( }, ); -export default compose( +export default memo( withOnyx({ userLocation: { key: ONYXKEYS.USER_LOCATION, }, - }), - memo, -)(MapView); + })(MapView), +); diff --git a/src/components/TestToolMenu.tsx b/src/components/TestToolMenu.tsx index e4d33957f7f1..85a2a975fc76 100644 --- a/src/components/TestToolMenu.tsx +++ b/src/components/TestToolMenu.tsx @@ -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 Navigation from '@libs/Navigation/Navigation'; import * as Network from '@userActions/Network'; import * as Session from '@userActions/Session'; @@ -109,11 +108,10 @@ function TestToolMenu({user = USER_DEFAULT, network}: TestToolMenuProps) { TestToolMenu.displayName = 'TestToolMenu'; -export default compose( +export default withNetwork()( withOnyx({ user: { key: ONYXKEYS.USER, }, - }), - withNetwork(), -)(TestToolMenu); + })(TestToolMenu), +); diff --git a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx index e5e203fb5030..123f7b12cb22 100644 --- a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx +++ b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx @@ -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'; @@ -103,7 +102,7 @@ export default function , OnyxProps>({ report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, @@ -138,9 +137,8 @@ export default function ({ - // @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM - reimbursementAccount: { - key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, - }, - workspaceRateAndUnit: { - key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT, - }, - }), - withPolicy, - withNetwork(), -)(WorkspaceRateAndUnitPage); +export default withNetwork()( + withPolicy( + withOnyx({ + // @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM + reimbursementAccount: { + key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, + }, + workspaceRateAndUnit: { + key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT, + }, + })(WorkspaceRateAndUnitPage), + ), +); diff --git a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/RatePage.tsx b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/RatePage.tsx index 8685cd3b1aee..e265e261b6b8 100644 --- a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/RatePage.tsx +++ b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/RatePage.tsx @@ -1,17 +1,16 @@ 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 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'; import CONST from '@src/CONST'; @@ -99,11 +98,10 @@ function WorkspaceRatePage(props: WorkspaceRatePageProps) { WorkspaceRatePage.displayName = 'WorkspaceRatePage'; -export default compose( +export default withPolicy( withOnyx({ workspaceRateAndUnit: { key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT, }, - }), - withPolicy, -)(WorkspaceRatePage); + })(WorkspaceRatePage), +); diff --git a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/UnitPage.tsx b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/UnitPage.tsx index 1d30c068e30d..6337fa2c461e 100644 --- a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/UnitPage.tsx +++ b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/UnitPage.tsx @@ -1,15 +1,14 @@ 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 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'; import CONST from '@src/CONST'; @@ -70,12 +69,10 @@ function WorkspaceUnitPage(props: WorkspaceUnitPageProps) { } WorkspaceUnitPage.displayName = 'WorkspaceUnitPage'; - -export default compose( +export default withPolicy( withOnyx({ workspaceRateAndUnit: { key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT, }, - }), - withPolicy, -)(WorkspaceUnitPage); + })(WorkspaceUnitPage), +); diff --git a/src/pages/workspace/withPolicyAndFullscreenLoading.tsx b/src/pages/workspace/withPolicyAndFullscreenLoading.tsx index 161320441843..2467136a382b 100644 --- a/src/pages/workspace/withPolicyAndFullscreenLoading.tsx +++ b/src/pages/workspace/withPolicyAndFullscreenLoading.tsx @@ -4,7 +4,6 @@ import React, {forwardRef} from 'react'; import type {OnyxEntry} from 'react-native-onyx'; import {withOnyx} from 'react-native-onyx'; import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; -import compose from '@libs/compose'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PersonalDetailsList} from '@src/types/onyx'; import type {WithPolicyOnyxProps, WithPolicyProps} from './withPolicy'; @@ -49,7 +48,7 @@ export default function withPolicyAndFullscreenLoading, WithPolicyAndFullscreenLoadingOnyxProps>({ isLoadingReportData: { key: ONYXKEYS.IS_LOADING_REPORT_DATA, @@ -57,9 +56,8 @@ export default function withPolicyAndFullscreenLoading Date: Mon, 13 May 2024 16:09:38 +0200 Subject: [PATCH 2/6] removing compose.ts --- src/libs/compose.ts | 71 --------------------------------------------- 1 file changed, 71 deletions(-) delete mode 100644 src/libs/compose.ts diff --git a/src/libs/compose.ts b/src/libs/compose.ts deleted file mode 100644 index dadc586d0f0d..000000000000 --- a/src/libs/compose.ts +++ /dev/null @@ -1,71 +0,0 @@ -/* eslint-disable @typescript-eslint/ban-types */ -/* eslint-disable import/export */ - -/** - * This is a utility function taken directly from Redux. (We don't want to add Redux as a dependency) - * It enables functional composition, useful for the chaining/composition of HOCs. - * - * For example, instead of: - * - * export default hoc1(config1, hoc2(config2, hoc3(config3)))(Component); - * - * Use this instead: - * - * export default compose( - * hoc1(config1), - * hoc2(config2), - * hoc3(config3), - * )(Component) - */ -export default function compose(): (a: R) => R; - -export default function compose(f: F): F; - -/* two functions */ -export default function compose(f1: (...args: A) => R1, f2: (a: R1) => R2): (...args: A) => R2; - -/* three functions */ -export default function compose(f1: (...args: A) => R1, f2: (a: R1) => R2, f3: (a: R2) => R3): (...args: A) => R3; - -/* four functions */ -export default function compose(f1: (...args: A) => R1, f2: (a: R1) => R2, f3: (a: R2) => R3, f4: (a: R3) => R4): (...args: A) => R4; - -/* five functions */ -export default function compose( - f1: (...args: A) => R1, - f2: (a: R1) => R2, - f3: (a: R2) => R3, - f4: (a: R3) => R4, - f5: (a: R4) => R5, -): (...args: A) => R5; - -/* six functions */ -export default function compose( - f1: (...args: A) => R1, - f2: (a: R1) => R2, - f3: (a: R2) => R3, - f4: (a: R3) => R4, - f5: (a: R4) => R5, - f6: (a: R5) => R6, -): (...args: A) => R6; - -/* rest */ -export default function compose(f1: (a: unknown) => R, ...funcs: Function[]): (...args: unknown[]) => R; - -export default function compose(...funcs: Function[]): Function { - if (funcs.length === 0) { - // infer the argument type so it is usable in inference down the line - return (arg: T) => arg; - } - - if (funcs.length === 1) { - return funcs[0]; - } - - return funcs.reduce( - (a, b) => - (...args: unknown[]) => - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - a(b(...args)), - ); -} From 6f730812c3d81f109873ef037c9f81a895ac3a6e Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Tue, 28 May 2024 16:12:07 +0200 Subject: [PATCH 3/6] Reversing dependencies order --- src/components/MapView/MapView.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/MapView/MapView.tsx b/src/components/MapView/MapView.tsx index 05a7cd221c1e..fa885160d881 100644 --- a/src/components/MapView/MapView.tsx +++ b/src/components/MapView/MapView.tsx @@ -195,10 +195,8 @@ const MapView = forwardRef( }, ); -export default memo( - withOnyx({ - userLocation: { - key: ONYXKEYS.USER_LOCATION, - }, - })(MapView), -); +export default withOnyx({ + userLocation: { + key: ONYXKEYS.USER_LOCATION, + }, +})(memo(MapView)); From ee1ff62e62fd5a2810e72efdca4b6f81f5b48c7f Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab <59809993+abzokhattab@users.noreply.github.com> Date: Thu, 13 Jun 2024 14:28:41 +0200 Subject: [PATCH 4/6] Removing composition section from documentation --- contributingGuides/STYLE.md | 46 ------------------------------------- 1 file changed, 46 deletions(-) diff --git a/contributingGuides/STYLE.md b/contributingGuides/STYLE.md index f3e928da8cb0..8c7729fa1807 100644 --- a/contributingGuides/STYLE.md +++ b/contributingGuides/STYLE.md @@ -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) @@ -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({ - session: { - key: ONYXKEYS.SESSION, - }, - }), - )(Component); - - // GOOD - export default withCurrentUserPersonalDetails( - withReportOrNotFound()( - withOnyx({ - session: { - key: ONYXKEYS.SESSION, - }, - })(Component), - ), - ); - - // GOOD - alternative to HOC nesting - const ComponentWithOnyx = withOnyx({ - 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. From fdf3e04e7f3620575fea62ba2b187a80ab8e2274 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Fri, 14 Jun 2024 16:53:56 +0200 Subject: [PATCH 5/6] removing withNetwork inside the initialPage --- .../WorkspaceRateAndUnitPage/InitialPage.tsx | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx index 7ccae2ac26b5..7b6aae7a0642 100644 --- a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx +++ b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx @@ -6,7 +6,6 @@ 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'; @@ -149,16 +148,14 @@ function WorkspaceRateAndUnitPage(props: WorkspaceRateAndUnitPageProps) { WorkspaceRateAndUnitPage.displayName = 'WorkspaceRateAndUnitPage'; -export default withNetwork()( - withPolicy( - withOnyx({ - // @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM - reimbursementAccount: { - key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, - }, - workspaceRateAndUnit: { - key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT, - }, - })(WorkspaceRateAndUnitPage), - ), +export default withPolicy( + withOnyx({ + // @ts-expect-error: ONYXKEYS.REIMBURSEMENT_ACCOUNT is conflicting with ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM + reimbursementAccount: { + key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, + }, + workspaceRateAndUnit: { + key: ONYXKEYS.WORKSPACE_RATE_AND_UNIT, + }, + })(WorkspaceRateAndUnitPage), ); From 8674e0d8e992993ed9ae451772dfb3ccc2800110 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Fri, 14 Jun 2024 19:01:54 +0200 Subject: [PATCH 6/6] initalpage props cleanup --- .../reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx index 7b6aae7a0642..243d1815bd92 100644 --- a/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx +++ b/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/InitialPage.tsx @@ -21,14 +21,11 @@ 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; -}; +type WorkspaceRateAndUnitPageBaseProps = WithPolicyProps; type WorkspaceRateAndUnitOnyxProps = { workspaceRateAndUnit: OnyxEntry;