[HOLD for payment 2025-01-18] [$250] Remove withCurrentReportID HOC

[luacmartins]

Coming from here, we should remove the withCurrentReportID HOC in favor of useCurrentReportID hook

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021869425871317443271
• Upwork Job ID: 1869425871317443271
• Last Price Increase: 2024-12-25
• Automatic offers:
• dominictb | Reviewer | 105506145
• etCoderDysto | Contributor | 105506146

Issue Owner

Current Issue Owner: @dominictb

[luacmartins]

Not a priority

[luacmartins]

Same

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021869425871317443271

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dominictb (External)

[etCoderDysto]

Edited by proposal-police: This proposal was edited at 2024-12-18 17:02:18 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Remove withCurrentReportID HOC

What is the root cause of that problem?

Refactor

What changes do you think we should make in order to solve the problem?

Replace every usage of withCurrentReportID hoc with useCurrentReportID hook

App/src/components/withCurrentReportID.tsx

Lines 64 to 69 in 804df09

	export default function withCurrentReportID<TProps extends CurrentReportIDContextValue, TRef>(
	WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
	): (props: Omit<TProps, keyof CurrentReportIDContextValue> & React.RefAttributes<TRef>) => React.ReactElement | null {
	function WithCurrentReportID(props: Omit<TProps, keyof CurrentReportIDContextValue>, ref: ForwardedRef<TRef>) {
	return (
	<CurrentReportIDContext.Consumer>

Places to replce the hoc
Report screen

App/src/pages/home/ReportScreen.tsx

Line 882 in 804df09

export default withCurrentReportID(memo(ReportScreen, (prevProps, nextProps) => prevProps.currentReportID === nextProps.currentReportID && lodashIsEqual(prevProps.route, nextProps.route)));

And we should check for all used palces

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

[github-actions]

cole Your proposal will be dismissed because you did not follow the proposal template.

[mkzie2]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Remove withCurrentReportID HOC

What is the root cause of that problem?

Improvement

What changes do you think we should make in order to solve the problem?

1. This HOC is only used in ReportScreen, we can replace this with useCurrentReportID hook and update the ReportScreenProps

const {currentReportID} = useCurrentReportID();

App/src/components/withCurrentReportID.tsx

Lines 64 to 69 in 804df09

	export default function withCurrentReportID<TProps extends CurrentReportIDContextValue, TRef>(
	WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
	): (props: Omit<TProps, keyof CurrentReportIDContextValue> & React.RefAttributes<TRef>) => React.ReactElement | null {
	function WithCurrentReportID(props: Omit<TProps, keyof CurrentReportIDContextValue>, ref: ForwardedRef<TRef>) {
	return (
	<CurrentReportIDContext.Consumer>

App/src/pages/home/ReportScreen.tsx

Line 882 in 17b3ef9

export default withCurrentReportID(memo(ReportScreen, (prevProps, nextProps) => prevProps.currentReportID === nextProps.currentReportID && lodashIsEqual(prevProps.route, nextProps.route)));

2. After that we can remove withCurrentReportID file and move all logic of CurrentReportIDContextProvider from withCurrentReportID file to useCurrentReportID file to centralize logic.

App/src/components/withCurrentReportID.tsx

Lines 64 to 69 in 0943fb6

	export default function withCurrentReportID<TProps extends CurrentReportIDContextValue, TRef>(
	WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
	): (props: Omit<TProps, keyof CurrentReportIDContextValue> & React.RefAttributes<TRef>) => React.ReactElement | null {
	function WithCurrentReportID(props: Omit<TProps, keyof CurrentReportIDContextValue>, ref: ForwardedRef<TRef>) {
	return (
	<CurrentReportIDContext.Consumer>

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

NA

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[dominictb]

on it now

[dominictb]

@etCoderDysto's proposal look good to me.

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @chiragsalian, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dominictb]

Moving the logic of CurrentReportIDContextProvider to useCurrentReportID can be a good idea, but it doesn't really matter. What do you think @chiragsalian?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[dominictb]

@chiragsalian @luacmartins can you please take a look at this decision when you have a chance?

[melvin-bot]

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @etCoderDysto 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[luacmartins]

Sorry, was ooo. Assinged @etCoderDysto to the issue

[etCoderDysto]

Thanks! Pr will be ready for review by tomorrow.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.83-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Remove withCurrentReportID HOC #54702

If no regressions arise, payment will be issued on 2025-01-18. 🎊

For reference, here are some details about the assignees on this issue:

• @dominictb requires payment automatic offer (Reviewer)
• @etCoderDysto requires payment automatic offer (Contributor)

[melvin-bot]

Issue is ready for payment but no BZ is assigned. @trjExpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

[trjExpensify]

Perf optimisation to remove an unused HOC, no regression test needed.

Payment summary as follows:

• $250 to @etCoderDysto for the PR
• $250 to @dominictb for the C+ review

Paid, closing!