[HOLD for payment 2024-05-02] [$250] [TS Migration] Remove underscore completely

[mountiny]

Follow up issue for TS migration project. Coming from this spreadsheet

Remove underscore completely

Should be straightforward once ready, needs to be tested well

cc @blazejkustra @fabioh8010

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014598e5194edc8278
• Upwork Job ID: 1780195244607991808
• Last Price Increase: 2024-04-16
• Automatic offers:
• hungvu193 | Reviewer | 0
• bernhardoj | Contributor | 0

[blazejkustra]

Details:

Simply remove underscore package along with all the usage in the codebase, instead use native methods or lodash as a last resort.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @MitchExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[mountiny]

Looking for proposlas on how to tackle this deprecation holistically, one or many PRs, how to divide them?

[ghost]

Dibs

[ghost]

Do we need to post a formal proposal for it?

[aneequeahmad]

I can take up this issue. i think one PR would be enough to fix it.

[mountiny]

Yes, please post how you want to approach this

[abzokhattab]

Proposal

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

Remove underscore completely

What is the root cause of that problem?

Remove underscore completely

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

Remove underscore completely from these files and replace it with the lodash lib.

• reportPropTypes
• MoneyTemporaryForRefactorRequestParticipantsSelector
• IOURequestStepRoutePropTypes
• MoneyRequestParticipantsSelector
• menuItemPropTypes
• transactionPropTypes
• BaseOptionsSelector
• popoverPropTypes

[nkdengineer]

Proposal

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

Remove the use of underscore

What is the root cause of that problem?

None

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

All files that are using underscore now are js file. We can remove underscore belong with migrate these files to typescript. If we update to use the lodash we still need another phrase to migrate these files to typescript. So I think the best way to do here is migrate these files as the image below to typescript.

What alternative solutions did you explore? (Optional)

NA

[bernhardoj]

Proposal

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

We want to remove underscore usage.

What is the root cause of that problem?

No root cause

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

There are 9 usages of underscore left.

6 out of 9 usages of underscore are being used in a prop types file and only transactionPropTypes and reportPropTypes that is still being used.

The plan is:

1. Delete unused prop types files
2. Replace underscore with JS native code

We can do it in 1 PR or 2 PR for each component file, BaseOptionsSelector and MoneyTemporaryForRefactorRequestParticipantSelector to reduce the scope for each PR.

The first PR will replace the underscore usages from MoneyTemporaryForRefactorRequestParticipantSelector, transactionPropTypes, and reportPropTypes and delete the unused prop types files.

The second PR will replace the underscore usages from BaseOptionsSelector.

MoneyRequestParticipantsSelector isn't being used anymore so we can skip it.

[hungvu193]

Thanks @blazejkustra.

[mountiny]

@hungvu193 all is migrated, just need to remove the underscore and then also remove the dependency. Can you check which proposals look the best for you?

[hungvu193]

@bernhardoj's proposal here LGTM.
Just a suggestion, I think 1 PR is enough for removing underscore usages. The second PR should remove the underscore dependency, such as eslint config, webpack config,..

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @mountiny is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @hungvu193 🎉 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]

📣 @bernhardoj 🎉 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 📖

[bernhardoj]

PR is here

[blazejkustra]

Just a suggestion, I think 1 PR is enough for removing underscore usages. The second PR should remove the underscore dependency, such as eslint config, webpack config,..

Why not just one PR that removes underscore completely?

[hungvu193]

Just a suggestion, I think 1 PR is enough for removing underscore usages. The second PR should remove the underscore dependency, such as eslint config, webpack config,..

Why not just one PR that removes underscore completely?

Oh I thought that will be a lots of changes if we include everything in a PR. But now I'm seeing that you're also reviewing it so I think that's fine with me.

[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 1.4.65-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Remove underscore usage #40346

If no regressions arise, payment will be issued on 2024-05-02. 🎊

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

• @hungvu193 requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@hungvu193] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@MitchExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[MitchExpensify]

Reminder set to Pay.

Payment summary:

$250 @hungvu193 requires payment automatic offer (Reviewer)
$250 @bernhardoj requires payment automatic offer (Contributor)

[MitchExpensify]

Paid and contracts ended