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.

Sign up for GitHub

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

Jump to bottom

Remove duplicate withOnyx calls #28866

Merged
merged 13 commits into from
Nov 10, 2023
Merged

Remove duplicate withOnyx calls #28866

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
33e8fdb
Remove some of the duplicate withOnyx calls
tgolen Oct 4, 2023
1478c41
Fix lint and remove console log
tgolen Oct 4, 2023
516dd4a
Merge branch 'main' into tgolen-remove-multiple-withOnyx
tgolen Oct 17, 2023
2253e4a
Remove some more nested references
tgolen Oct 17, 2023
5a93801
Simplify withOnyx keys
tgolen Oct 17, 2023
21dbdb7
Simplify another key
tgolen Oct 17, 2023
f93bfc5
Merge branch 'main' into tgolen-remove-multiple-withOnyx
tgolen Nov 6, 2023
bd4e4a5
Hide most output when running prettier
tgolen Nov 6, 2023
31ec933
Remove a duplicate withOnyx
tgolen Nov 6, 2023
0431e48
Remove unused import
tgolen Nov 6, 2023
29a510b
Merge branch 'main' into tgolen-remove-multiple-withOnyx
tgolen Nov 7, 2023
4547f97
Merge branch 'main' into tgolen-remove-multiple-withOnyx
tgolen Nov 8, 2023
346e6b6
Return 0 instead of an empty string
tgolen Nov 9, 2023
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
1 change: 0 additions & 1 deletion .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ module.exports = {
files: ['*.js', '*.jsx', '*.ts', '*.tsx'],
plugins: ['react'],
rules: {
'rulesdir/no-multiple-onyx-in-file': 'off',
'rulesdir/onyx-props-must-have-default': 'off',
'react-native-a11y/has-accessibility-hint': ['off'],
'react/jsx-no-constructed-context-values': 'error',
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"lint-changed": "eslint --fix $(git diff --diff-filter=AM --name-only main -- \"*.js\" \"*.ts\" \"*.tsx\")",
"lint-watch": "npx eslint-watch --watch --changed",
"shellcheck": "./scripts/shellCheck.sh",
"prettier": "prettier --write .",
"prettier": "prettier --write . --loglevel warn",
"prettier-watch": "onchange \"**/*.{js,ts,tsx}\" -- prettier --write --ignore-unknown {{changed}}",
"print-version": "echo $npm_package_version",
"storybook": "start-storybook -p 6006",
Expand Down
40 changes: 17 additions & 23 deletions src/components/MoneyRequestHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useWindowDimensions from '@hooks/useWindowDimensions';
import compose from '@libs/compose';
import * as HeaderUtils from '@libs/HeaderUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
Expand Down Expand Up @@ -162,26 +161,21 @@ MoneyRequestHeader.displayName = 'MoneyRequestHeader';
MoneyRequestHeader.propTypes = propTypes;
MoneyRequestHeader.defaultProps = defaultProps;

export default compose(
withOnyx({
session: {
key: ONYXKEYS.SESSION,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`,
canEvict: false,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({report, parentReportActions}) => {
const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID]);
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
export default withOnyx({
session: {
key: ONYXKEYS.SESSION,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : 0}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : 0}`,
canEvict: false,
},
transaction: {
key: ({report, parentReportActions}) => {
const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID]);
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
}),
)(MoneyRequestHeader);
},
})(MoneyRequestHeader);
63 changes: 27 additions & 36 deletions src/pages/EditRequestPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import categoryPropTypes from '@components/categoryPropTypes';
import ScreenWrapper from '@components/ScreenWrapper';
import tagPropTypes from '@components/tagPropTypes';
import transactionPropTypes from '@components/transactionPropTypes';
import compose from '@libs/compose';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as OptionsListUtils from '@libs/OptionsListUtils';
Expand Down Expand Up @@ -276,39 +275,31 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories,
EditRequestPage.displayName = 'EditRequestPage';
EditRequestPage.propTypes = propTypes;
EditRequestPage.defaultProps = defaultProps;
export default compose(
withOnyx({
betas: {
key: ONYXKEYS.BETAS,
export default withOnyx({
betas: {
key: ONYXKEYS.BETAS,
},
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.threadReportID}`,
},
policyCategories: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${report ? report.policyID : '0'}`,
},
policyTags: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report ? report.policyID : '0'}`,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '0'}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`,
canEvict: false,
},
transaction: {
key: ({report, parentReportActions}) => {
const parentReportActionID = lodashGet(report, 'parentReportActionID', '0');
const parentReportAction = lodashGet(parentReportActions, parentReportActionID);
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.threadReportID}`,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
policyCategories: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${report ? report.policyID : '0'}`,
},
policyTags: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report ? report.policyID : '0'}`,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '0'}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`,
canEvict: false,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({report, parentReportActions}) => {
const parentReportActionID = lodashGet(report, 'parentReportActionID', '0');
const parentReportAction = lodashGet(parentReportActions, parentReportActionID);
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
},
}),
)(EditRequestPage);
},
})(EditRequestPage);
38 changes: 16 additions & 22 deletions src/pages/EditSplitBillPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import React from 'react';
import {withOnyx} from 'react-native-onyx';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import transactionPropTypes from '@components/transactionPropTypes';
import compose from '@libs/compose';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as ReportUtils from '@libs/ReportUtils';
Expand Down Expand Up @@ -136,26 +135,21 @@ function EditSplitBillPage({route, transaction, draftTransaction}) {
EditSplitBillPage.displayName = 'EditSplitBillPage';
EditSplitBillPage.propTypes = propTypes;
EditSplitBillPage.defaultProps = defaultProps;
export default compose(
withOnyx({
reportActions: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${route.params.reportID}`,
canEvict: false,
export default withOnyx({
reportActions: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${route.params.reportID}`,
canEvict: false,
},
transaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions[`${route.params.reportActionID.toString()}`];
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(reportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions[`${route.params.reportActionID.toString()}`];
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(reportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
},
draftTransaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions[`${route.params.reportActionID.toString()}`];
return `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${lodashGet(reportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
draftTransaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions[`${route.params.reportActionID.toString()}`];
return `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${lodashGet(reportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
},
}),
)(EditSplitBillPage);
},
})(EditSplitBillPage);
26 changes: 8 additions & 18 deletions src/pages/iou/MoneyRequestCategoryPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import compose from '@libs/compose';
import Navigation from '@libs/Navigation/Navigation';
import reportPropTypes from '@pages/reportPropTypes';
import styles from '@styles/styles';
Expand Down Expand Up @@ -86,20 +85,11 @@ MoneyRequestCategoryPage.displayName = 'MoneyRequestCategoryPage';
MoneyRequestCategoryPage.propTypes = propTypes;
MoneyRequestCategoryPage.defaultProps = defaultProps;

export default compose(
withOnyx({
iou: {
key: ONYXKEYS.IOU,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
report: {
key: ({route, iou}) => {
const reportID = IOU.getIOUReportID(iou, route);

return `${ONYXKEYS.COLLECTION.REPORT}${reportID}`;
},
},
}),
)(MoneyRequestCategoryPage);
export default withOnyx({
iou: {
key: ONYXKEYS.IOU,
},
report: {
key: ({route, iou}) => `${ONYXKEYS.COLLECTION.REPORT}${IOU.getIOUReportID(iou, route)}`,
},
})(MoneyRequestCategoryPage);
35 changes: 11 additions & 24 deletions src/pages/iou/MoneyRequestTagPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import TagPicker from '@components/TagPicker';
import tagPropTypes from '@components/tagPropTypes';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import compose from '@libs/compose';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import reportPropTypes from '@pages/reportPropTypes';
Expand Down Expand Up @@ -96,26 +95,14 @@ MoneyRequestTagPage.displayName = 'MoneyRequestTagPage';
MoneyRequestTagPage.propTypes = propTypes;
MoneyRequestTagPage.defaultProps = defaultProps;

export default compose(
withOnyx({
iou: {
key: ONYXKEYS.IOU,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
report: {
key: ({route, iou}) => {
const reportID = IOU.getIOUReportID(iou, route);

return `${ONYXKEYS.COLLECTION.REPORT}${reportID}`;
},
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
policyTags: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report ? report.policyID : '0'}`,
},
}),
)(MoneyRequestTagPage);
export default withOnyx({
report: {
key: ({route, iou}) => `${ONYXKEYS.COLLECTION.REPORT}${IOU.getIOUReportID(iou, route)}`,
},
iou: {
key: ONYXKEYS.IOU,
},
Comment on lines +99 to +104
Copy link
Contributor

@situchan situchan Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I logged iou here and noticed a slight difference.

    report: {
        key: ({route, iou}) => {
            console.log('iou', iou)
            return `${ONYXKEYS.COLLECTION.REPORT}${IOU.getIOUReportID(iou, route)}`
         }
    },

Multiple withOnyx:

Screenshot 2023-11-09 at 10 18 59 PM

Single withOnyx:

Screenshot 2023-11-09 at 10 17 07 PM

As you see, iou is undefined first time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's interesting. The undefined is expected so the only thing that matters is that getIOUReport() protects against an undefined value, which it does. However, I see that it returns an empty string in the case of undefined which is not desired and would subscribe to the entire report collection. I've updated this to return 0 for the iou report ID which is the more proper way of handling this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am still concerned about undefined on first render.
Sometimes this will cause unexpected behavior like #30866 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree with you that it's a problem for #30866. I would like to take a look at it in that PR and try to solve it over there. I feel it's unrelated to this specifically, but will let you know.

policyTags: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report ? report.policyID : '0'}`,
},
})(MoneyRequestTagPage);
3 changes: 0 additions & 3 deletions src/pages/iou/SplitBillDetailsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,6 @@ export default compose(
session: {
key: ONYXKEYS.SESSION,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions[`${route.params.reportActionID.toString()}`];
Expand Down
6 changes: 0 additions & 6 deletions src/pages/iou/steps/MoneyRequestConfirmPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,9 +411,6 @@ export default compose(
iou: {
key: ONYXKEYS.IOU,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
report: {
key: ({route, iou}) => {
const reportID = IOU.getIOUReportID(iou, route);
Expand All @@ -427,9 +424,6 @@ export default compose(
selectedTab: {
key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.RECEIPT_TAB_ID}`,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
policy: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY}${report ? report.policyID : '0'}`,
},
Expand Down
28 changes: 11 additions & 17 deletions src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import ScreenWrapper from '@components/ScreenWrapper';
import transactionPropTypes from '@components/transactionPropTypes';
import useInitialValue from '@hooks/useInitialValue';
import useLocalize from '@hooks/useLocalize';
import compose from '@libs/compose';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import * as MoneyRequestUtils from '@libs/MoneyRequestUtils';
import Navigation from '@libs/Navigation/Navigation';
Expand Down Expand Up @@ -155,19 +154,14 @@ MoneyRequestParticipantsPage.displayName = 'MoneyRequestParticipantsPage';
MoneyRequestParticipantsPage.propTypes = propTypes;
MoneyRequestParticipantsPage.defaultProps = defaultProps;

export default compose(
withOnyx({
iou: {
key: ONYXKEYS.IOU,
},
selectedTab: {
key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.RECEIPT_TAB_ID}`,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({iou}) => `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(iou, 'transactionID', 0)}`,
},
}),
)(MoneyRequestParticipantsPage);
export default withOnyx({
iou: {
key: ONYXKEYS.IOU,
},
selectedTab: {
key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.RECEIPT_TAB_ID}`,
},
transaction: {
key: ({iou}) => `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(iou, 'transactionID', 0)}`,
},
})(MoneyRequestParticipantsPage);
Loading