From 8357fb92d1e27998afaf81146f3a179262cac66c Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Mon, 16 Oct 2023 08:48:32 +0200 Subject: [PATCH 1/9] start migrating the file, update eslint rules --- .eslintrc.js | 2 +- ...OrNotFound.js => withReportOrNotFound.tsx} | 41 ++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) rename src/pages/home/report/{withReportOrNotFound.js => withReportOrNotFound.tsx} (71%) diff --git a/.eslintrc.js b/.eslintrc.js index 75a74ed371c4..83e9479ce0c4 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -116,7 +116,7 @@ module.exports = { }, { selector: ['parameter', 'method'], - format: ['camelCase'], + format: ['camelCase', 'PascalCase'], }, ], '@typescript-eslint/ban-types': [ diff --git a/src/pages/home/report/withReportOrNotFound.js b/src/pages/home/report/withReportOrNotFound.tsx similarity index 71% rename from src/pages/home/report/withReportOrNotFound.js rename to src/pages/home/report/withReportOrNotFound.tsx index 5829ac7a6015..ad9a44528384 100644 --- a/src/pages/home/report/withReportOrNotFound.js +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React from 'react'; +import React, {ComponentType, ForwardedRef, RefAttributes} from 'react'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import getComponentDisplayName from '../../../libs/getComponentDisplayName'; @@ -8,8 +8,37 @@ import ONYXKEYS from '../../../ONYXKEYS'; import reportPropTypes from '../../reportPropTypes'; import FullscreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import * as ReportUtils from '../../../libs/ReportUtils'; - -export default function (WrappedComponent) { +import * as OnyxTypes from '../../../types/onyx'; + +// export default function withNavigation(WrappedComponent: ComponentType>) { +// function WithNavigation(props: Omit, ref: ForwardedRef) { +// const navigation = useNavigation(); +// return ( +// +// ); +// } + +// WithNavigation.displayName = `withNavigation(${getComponentDisplayName(WrappedComponent)})`; +// return React.forwardRef(WithNavigation); +// } + +type WithReportOrNotFoundProps = { + /** The report currently being looked at */ + report: OnyxTypes.Report; + /** The policies which the user has access to */ + policies: OnyxTypes.Policy; + /** Beta features list */ + betas: OnyxTypes.Beta[]; + /** Indicated whether the report data is loading */ + isLoadingReportData: boolean; +}; + +export default function (WrappedComponent: ComponentType>) { const propTypes = { /** The HOC takes an optional ref as a prop and passes it as a ref to the wrapped component. * That way, if a ref is passed to a component wrapped in the HOC, the ref is a reference to the wrapped component, not the HOC. */ @@ -45,7 +74,9 @@ export default function (WrappedComponent) { }; // eslint-disable-next-line rulesdir/no-negated-variables - function WithReportOrNotFound(props) { + function WithReportOrNotFound(props: WithReportOrNotFoundProps, ref: ForwardedRef) { + console.log('***********!!!!!************'); + const contentShown = React.useRef(false); const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData && (_.isEmpty(props.report) || !props.report.reportID); @@ -75,7 +106,7 @@ export default function (WrappedComponent) { ); } From c433fcf3f8f3199efd5dd5471f675d1b8349218b Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Tue, 17 Oct 2023 08:56:13 +0200 Subject: [PATCH 2/9] remove prop types --- .../home/report/withReportOrNotFound.tsx | 51 ++----------------- 1 file changed, 4 insertions(+), 47 deletions(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index ad9a44528384..9414a6d07b80 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -39,40 +39,6 @@ type WithReportOrNotFoundProps = { }; export default function (WrappedComponent: ComponentType>) { - const propTypes = { - /** The HOC takes an optional ref as a prop and passes it as a ref to the wrapped component. - * That way, if a ref is passed to a component wrapped in the HOC, the ref is a reference to the wrapped component, not the HOC. */ - forwardedRef: PropTypes.func, - - /** The report currently being looked at */ - report: reportPropTypes, - - /** The policies which the user has access to */ - policies: PropTypes.objectOf( - PropTypes.shape({ - /** The policy name */ - name: PropTypes.string, - - /** The type of the policy */ - type: PropTypes.string, - }), - ), - - /** Beta features list */ - betas: PropTypes.arrayOf(PropTypes.string), - - /** Indicated whether the report data is loading */ - isLoadingReportData: PropTypes.bool, - }; - - const defaultProps = { - forwardedRef: () => {}, - report: {}, - policies: {}, - betas: [], - isLoadingReportData: true, - }; - // eslint-disable-next-line rulesdir/no-negated-variables function WithReportOrNotFound(props: WithReportOrNotFoundProps, ref: ForwardedRef) { console.log('***********!!!!!************'); @@ -101,29 +67,20 @@ export default function (Wrapped contentShown.current = true; } - const rest = _.omit(props, ['forwardedRef']); + // const rest = _.omit(props, ['forwardedRef']); return ( ); } - WithReportOrNotFound.propTypes = propTypes; - WithReportOrNotFound.defaultProps = defaultProps; - WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`; - // eslint-disable-next-line rulesdir/no-negated-variables - const withReportOrNotFound = React.forwardRef((props, ref) => ( - - )); + const withReportOrNotFound = React.forwardRef(WithReportOrNotFound); + WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`; return withOnyx({ report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, From a3b2bac4b6412dc3d0887b73988365bfbcf62888 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Tue, 17 Oct 2023 14:24:55 +0200 Subject: [PATCH 3/9] fix most of the typings --- src/libs/ReportUtils.js | 4 +- src/libs/getComponentDisplayName.ts | 2 +- .../home/report/withReportOrNotFound.tsx | 59 +++++++++---------- 3 files changed, 30 insertions(+), 35 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index c04c71fbd625..d8cd40ec091f 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3018,8 +3018,8 @@ function canSeeDefaultRoom(report, policies, betas) { /** * @param {Object} report - * @param {Array} policies - * @param {Array} betas + * @param {Object | null} policies + * @param {Array | null} betas * @param {Object} allReportActions * @returns {Boolean} */ diff --git a/src/libs/getComponentDisplayName.ts b/src/libs/getComponentDisplayName.ts index fd1bbcaea521..0bf52d543a84 100644 --- a/src/libs/getComponentDisplayName.ts +++ b/src/libs/getComponentDisplayName.ts @@ -1,6 +1,6 @@ import {ComponentType} from 'react'; /** Returns the display name of a component */ -export default function getComponentDisplayName(component: ComponentType): string { +export default function getComponentDisplayName(component: ComponentType): string { return component.displayName ?? component.name ?? 'Component'; } diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index 9414a6d07b80..90ea9d5abd82 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -1,53 +1,47 @@ -import PropTypes from 'prop-types'; import React, {ComponentType, ForwardedRef, RefAttributes} from 'react'; -import {withOnyx} from 'react-native-onyx'; -import _ from 'underscore'; +import {OnyxEntry, withOnyx} from 'react-native-onyx'; import getComponentDisplayName from '../../../libs/getComponentDisplayName'; import NotFoundPage from '../../ErrorPage/NotFoundPage'; import ONYXKEYS from '../../../ONYXKEYS'; -import reportPropTypes from '../../reportPropTypes'; import FullscreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import * as ReportUtils from '../../../libs/ReportUtils'; import * as OnyxTypes from '../../../types/onyx'; -// export default function withNavigation(WrappedComponent: ComponentType>) { -// function WithNavigation(props: Omit, ref: ForwardedRef) { -// const navigation = useNavigation(); -// return ( -// -// ); -// } - -// WithNavigation.displayName = `withNavigation(${getComponentDisplayName(WrappedComponent)})`; -// return React.forwardRef(WithNavigation); -// } - -type WithReportOrNotFoundProps = { +type OnyxProps = { /** The report currently being looked at */ - report: OnyxTypes.Report; + report: OnyxEntry; /** The policies which the user has access to */ - policies: OnyxTypes.Policy; + // policies: OnyxTypes.Policy[]; + policies: OnyxEntry; /** Beta features list */ - betas: OnyxTypes.Beta[]; + // betas: OnyxTypes.Beta[]; + betas: OnyxEntry; /** Indicated whether the report data is loading */ - isLoadingReportData: boolean; + isLoadingReportData: OnyxEntry; }; -export default function (WrappedComponent: ComponentType>) { +type RouteProps = { + route: { + params: { + reportID: string; + }; + }; +}; + +type HOCProps = {}; + +type ComponentProps = OnyxProps & HOCProps; + +export default function (WrappedComponent: ComponentType>) { // eslint-disable-next-line rulesdir/no-negated-variables - function WithReportOrNotFound(props: WithReportOrNotFoundProps, ref: ForwardedRef) { + function WithReportOrNotFound(props: Omit, ref: ForwardedRef) { console.log('***********!!!!!************'); const contentShown = React.useRef(false); - const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData && (_.isEmpty(props.report) || !props.report.reportID); + const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData && (!Object.entries(props.report ?? {}).length || !props.report?.reportID); // eslint-disable-next-line rulesdir/no-negated-variables - const shouldShowNotFoundPage = _.isEmpty(props.report) || !props.report.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas); + const shouldShowNotFoundPage = _.isEmpty(props.report) || !props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas, {}); // If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen. // Return null for this case to avoid rendering FullScreenLoadingIndicator or NotFoundPage when animating transition. @@ -77,11 +71,12 @@ export default function (Wrapped ); } + WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`; + // eslint-disable-next-line rulesdir/no-negated-variables const withReportOrNotFound = React.forwardRef(WithReportOrNotFound); - WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`; - return withOnyx({ + return withOnyx & RefAttributes & RouteProps, OnyxProps>({ report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, }, From 4983523b6f248cb38602d6953fec9698da5c0252 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Tue, 17 Oct 2023 14:26:47 +0200 Subject: [PATCH 4/9] remove underscore --- src/pages/home/report/withReportOrNotFound.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index 90ea9d5abd82..56eb603a0f6d 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -36,12 +36,11 @@ export default function (WrappedComponent: // eslint-disable-next-line rulesdir/no-negated-variables function WithReportOrNotFound(props: Omit, ref: ForwardedRef) { console.log('***********!!!!!************'); - const contentShown = React.useRef(false); const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData && (!Object.entries(props.report ?? {}).length || !props.report?.reportID); // eslint-disable-next-line rulesdir/no-negated-variables - const shouldShowNotFoundPage = _.isEmpty(props.report) || !props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas, {}); + const shouldShowNotFoundPage = !Object.entries(props.report ?? {}).length || !props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas, {}); // If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen. // Return null for this case to avoid rendering FullScreenLoadingIndicator or NotFoundPage when animating transition. From 6896d5d1fde6ac5b557e41c9430fcccde8c89ab2 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Tue, 17 Oct 2023 14:59:50 +0200 Subject: [PATCH 5/9] fix all typings --- src/pages/home/report/withReportOrNotFound.tsx | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index 56eb603a0f6d..3eec0105d3ec 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -28,14 +28,9 @@ type RouteProps = { }; }; -type HOCProps = {}; - -type ComponentProps = OnyxProps & HOCProps; - -export default function (WrappedComponent: ComponentType>) { +export default function (WrappedComponent: ComponentType>) { // eslint-disable-next-line rulesdir/no-negated-variables - function WithReportOrNotFound(props: Omit, ref: ForwardedRef) { - console.log('***********!!!!!************'); + function WithReportOrNotFound(props: TProps, ref: ForwardedRef) { const contentShown = React.useRef(false); const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData && (!Object.entries(props.report ?? {}).length || !props.report?.reportID); @@ -60,11 +55,10 @@ export default function (WrappedComponent: contentShown.current = true; } - // const rest = _.omit(props, ['forwardedRef']); return ( ); @@ -75,7 +69,7 @@ export default function (WrappedComponent: // eslint-disable-next-line rulesdir/no-negated-variables const withReportOrNotFound = React.forwardRef(WithReportOrNotFound); - return withOnyx & RefAttributes & RouteProps, OnyxProps>({ + return withOnyx & RouteProps, OnyxProps>({ report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, }, From a66c1219b0cdc20c82296880ea8061e60f07fb9b Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Tue, 17 Oct 2023 15:15:30 +0200 Subject: [PATCH 6/9] remove unnecessary comments --- src/pages/home/report/withReportOrNotFound.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index 3eec0105d3ec..5795120a9c8e 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -11,10 +11,8 @@ type OnyxProps = { /** The report currently being looked at */ report: OnyxEntry; /** The policies which the user has access to */ - // policies: OnyxTypes.Policy[]; policies: OnyxEntry; /** Beta features list */ - // betas: OnyxTypes.Beta[]; betas: OnyxEntry; /** Indicated whether the report data is loading */ isLoadingReportData: OnyxEntry; From 1b5cb6454e742d41618c4bd88b997861cf937cc5 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Wed, 18 Oct 2023 09:24:13 +0200 Subject: [PATCH 7/9] improve route props typing --- src/pages/home/report/withReportOrNotFound.tsx | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index 5795120a9c8e..cfea480a59a8 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -6,6 +6,7 @@ import ONYXKEYS from '../../../ONYXKEYS'; import FullscreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import * as ReportUtils from '../../../libs/ReportUtils'; import * as OnyxTypes from '../../../types/onyx'; +import {RouteProp} from '@react-navigation/native'; type OnyxProps = { /** The report currently being looked at */ @@ -18,15 +19,11 @@ type OnyxProps = { isLoadingReportData: OnyxEntry; }; -type RouteProps = { - route: { - params: { - reportID: string; - }; - }; +type ComponentProps = OnyxProps & { + route: RouteProp<{params: {reportID: string}}>; }; -export default function (WrappedComponent: ComponentType>) { +export default function (WrappedComponent: ComponentType>) { // eslint-disable-next-line rulesdir/no-negated-variables function WithReportOrNotFound(props: TProps, ref: ForwardedRef) { const contentShown = React.useRef(false); @@ -67,7 +64,7 @@ export default function (WrappedComponent: Compo // eslint-disable-next-line rulesdir/no-negated-variables const withReportOrNotFound = React.forwardRef(WithReportOrNotFound); - return withOnyx & RouteProps, OnyxProps>({ + return withOnyx, OnyxProps>({ report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, }, From df3e7d7a234cc6385938832b70a7771960dd2b11 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Wed, 18 Oct 2023 10:01:05 +0200 Subject: [PATCH 8/9] fix imports order --- src/pages/home/report/withReportOrNotFound.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index cfea480a59a8..46e7dd5cbc0f 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -1,12 +1,12 @@ import React, {ComponentType, ForwardedRef, RefAttributes} from 'react'; import {OnyxEntry, withOnyx} from 'react-native-onyx'; +import {RouteProp} from '@react-navigation/native'; import getComponentDisplayName from '../../../libs/getComponentDisplayName'; import NotFoundPage from '../../ErrorPage/NotFoundPage'; import ONYXKEYS from '../../../ONYXKEYS'; import FullscreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import * as ReportUtils from '../../../libs/ReportUtils'; import * as OnyxTypes from '../../../types/onyx'; -import {RouteProp} from '@react-navigation/native'; type OnyxProps = { /** The report currently being looked at */ From 6dae6fcffb53ab9868661b5ad0f59c5ab3fead3a Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Fri, 20 Oct 2023 14:02:31 +0200 Subject: [PATCH 9/9] apply changes based of feedback from code review --- src/pages/home/report/withReportOrNotFound.tsx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/pages/home/report/withReportOrNotFound.tsx b/src/pages/home/report/withReportOrNotFound.tsx index 46e7dd5cbc0f..f1a855823c5e 100644 --- a/src/pages/home/report/withReportOrNotFound.tsx +++ b/src/pages/home/report/withReportOrNotFound.tsx @@ -1,3 +1,4 @@ +/* eslint-disable rulesdir/no-negated-variables */ import React, {ComponentType, ForwardedRef, RefAttributes} from 'react'; import {OnyxEntry, withOnyx} from 'react-native-onyx'; import {RouteProp} from '@react-navigation/native'; @@ -23,13 +24,14 @@ type ComponentProps = OnyxProps & { route: RouteProp<{params: {reportID: string}}>; }; -export default function (WrappedComponent: ComponentType>) { - // eslint-disable-next-line rulesdir/no-negated-variables +export default function ( + WrappedComponent: ComponentType>, +): React.ComponentType, keyof OnyxProps>> { function WithReportOrNotFound(props: TProps, ref: ForwardedRef) { const contentShown = React.useRef(false); const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData && (!Object.entries(props.report ?? {}).length || !props.report?.reportID); - // eslint-disable-next-line rulesdir/no-negated-variables + const shouldShowNotFoundPage = !Object.entries(props.report ?? {}).length || !props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas, {}); // If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen. @@ -61,9 +63,6 @@ export default function (WrappedComponent: WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`; - // eslint-disable-next-line rulesdir/no-negated-variables - const withReportOrNotFound = React.forwardRef(WithReportOrNotFound); - return withOnyx, OnyxProps>({ report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`, @@ -77,5 +76,5 @@ export default function (WrappedComponent: policies: { key: ONYXKEYS.COLLECTION.POLICY, }, - })(withReportOrNotFound); + })(React.forwardRef(WithReportOrNotFound)); }