perf: use requiresAttentionFromCurrentUser from a derived value #61586
+46
−32
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -707,7 +707,6 @@ type OptionData = { | |
| alternateText?: string; | ||
| allReportErrors?: Errors; | ||
| brickRoadIndicator?: ValueOf<typeof CONST.BRICK_ROAD_INDICATOR_STATUS> | '' | null; | ||
| tooltipText?: string | null; | ||
| alternateTextMaxLines?: number; | ||
| boldStyle?: boolean; | ||
|
|
@@ -1059,14 +1058,14 @@ Onyx.connect({ | |
| callback: (value) => (activePolicyID = value), | ||
| }); | ||
|
|
||
| let reportAttributesDerivedValue: ReportAttributesDerivedValue['reports']; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, | ||
| callback: (value) => { | ||
| if (!value) { | ||
| return; | ||
| } | ||
| reportAttributesDerivedValue = value.reports; | ||
| }, | ||
| }); | ||
|
|
||
|
|
@@ -4647,11 +4646,11 @@ function getReportName( | |
| parentReportActionParam?: OnyxInputOrEntry<ReportAction>, | ||
| personalDetails?: Partial<PersonalDetailsList>, | ||
| invoiceReceiverPolicy?: OnyxEntry<Policy>, | ||
| reportAttributes?: ReportAttributesDerivedValue['reports'], | ||
| ): string { | ||
| // Check if we can use report name in derived values - only when we have report but no other params | ||
| const canUseDerivedValue = report && policy === undefined && parentReportActionParam === undefined && personalDetails === undefined && invoiceReceiverPolicy === undefined; | ||
| const attributes = reportAttributes ?? reportAttributesDerivedValue; | ||
| const derivedNameExists = report && !!attributes?.[report.reportID]?.reportName; | ||
| if (canUseDerivedValue && derivedNameExists) { | ||
| return attributes[report.reportID].reportName; | ||
|
|
@@ -10754,6 +10753,15 @@ function getReportPersonalDetailsParticipants(report: Report, personalDetailsPar | |
| }; | ||
| } | ||
|
|
||
| function getReportAttributes(reportID: string | undefined, reportAttributes?: ReportAttributesDerivedValue['reports']) { | ||
| const attributes = reportAttributes ?? reportAttributesDerivedValue; | ||
|
|
||
| if (!reportID || !attributes?.[reportID]) { | ||
| return; | ||
| } | ||
| return attributes[reportID]; | ||
| } | ||
|
|
||
|
Comment on lines
+10756
to
+10764
There was a problem hiding this comment. I think we dont want to create these getters in general and always use the attributes fetched from onyx in the component There was a problem hiding this comment. Addressed this comment here: #62464 |
||
| export { | ||
| addDomainToShortMention, | ||
| completeShortMention, | ||
|
|
@@ -11130,6 +11138,7 @@ export { | |
| getReportPersonalDetailsParticipants, | ||
| isAllowedToSubmitDraftExpenseReport, | ||
| isWorkspaceEligibleForReportChange, | ||
| getReportAttributes, | ||
| }; | ||
|
|
||
| export type { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,13 +73,13 @@ export default createOnyxDerivedValueConfig({ | |
| dataToIterate = prepareReportKeys(updates); | ||
| recentlyUpdated = updates; | ||
| } else if (!!transactionsUpdates || !!transactionViolationsUpdates) { | ||
| let transactionReportIDs: string[] = []; | ||
|
There was a problem hiding this comment. Addressing this NAB comment from previous PR #61376 (comment) here cc @mountiny |
||
| if (transactionsUpdates) { | ||
| transactionReportIDs = Object.values(transactionsUpdates).map((transaction) => `${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`); | ||
| } | ||
| // if transactions are updated, they might not be directly related to the reports yet (e.g. transaction is optimistically created) | ||
| // so we use report keys that were updated before to recompute the reports | ||
| const recentReportKeys = prepareReportKeys([...recentlyUpdated, ...transactionReportIDs]); | ||
| dataToIterate = recentReportKeys; | ||
| } | ||
| } | ||
|
|
@@ -112,6 +112,7 @@ export default createOnyxDerivedValueConfig({ | |
| acc[report.reportID] = { | ||
| reportName: generateReportName(report), | ||
| brickRoadStatus, | ||
| requiresAttention, | ||
| }; | ||
|
|
||
| return acc; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is an antipattern now and we should always subscribe to the attributes in the component using useOnyx and pass them to the utils method - this should ensure the component is always re-rendered when the attribute changes and that the utils method remains pure.
@TMisiukiewicz @kacper-mikolajczak what do you think? Could you look into this to refactor its usages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it should be passed to the util methods from
useOnyx. The only caveat is that in some cases, those values might be used deep within the function logic, not just as top-level arguments. Refactoring might result in passing them through multiple layers of intermediate calls.I'll verify if it'll introduce the additional complexity