[HOLD for payment 2022-04-12] Move `user.loginList` data to new, specialized ONYX key

[Beamanator]

Problem

A user's loginList data is currently stored in the USER Onyx key, among lots of other user-related data. This is not ideal, since any time this key is updated, every page that subscribes to the USER onyx key will re-render (unless we specifically prevent it using componentShouldUpdate and checking prev vs current props).

Why is this important?

Unnecessary re-renders makes the app slower and can make the app behave unexpectedly so it's always best to limit the amount of rendering.

Solution

While reviewing the N7 Free Plan Account Settings doc (internal), we realized it would be better in the future to have the user's loginList separated from the USER Onyx key into its own key, like LOGIN_LIST. This way we don't have to think if some user data changed that isn't being displayed on each page subscribing to onyx data.

cc @tgolen since this was your idea :D

[MelvinBot]

Triggered auto assignment to @dylanexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[dylanexpensify]

On it!

[dylanexpensify]

Internal: https://www.upwork.com/ab/applicants/1501589596015407104/job-details External: https://www.upwork.com/jobs/~01466c5a04465e487c

[MelvinBot]

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

[MelvinBot]

Current assignee @Beamanator is eligible for the Exported assigner, not assigning anyone new.

[mdneyazahmad]

Proposal

As the solution is to only refactor the code. The diff is little big so I am posting here you can review my proposal from https://github.com/mdneyazahmad/App/tree/refactor/8051-onyx-keys

[parasharrajat]

I don't think there is some specific strategy required for this issue. The solution requires a straight refactor of code and @mdneyazahmad is the first to show interest so I am fine with him taking up this task.

Few Notes:

1. Please use USER_LOGIN_LIST key name.
2. Carefully do the clean-up of USER subscription like you did in one of the files in your changes.
3. Remember to test every functionality carefully that your PR touches before requesting the PR review.

cc: @Beamanator

:ribbon: :eyes: :ribbon: C+ reviewed

[mdneyazahmad]

@parasharrajat Thanks for your review. I think this was the only changes required for this issue. But, I will test it throughly before creating a pr.

[MelvinBot]

📣 @mdneyazahmad You have been assigned to this job by @Beamanator! Please apply to this job in Upwork 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 📖

[Beamanator]

Thanks for the review @parasharrajat and thanks for the sample @mdneyazahmad 👍

[dylanexpensify]

Reassigning as I'm OOO next week!

[MelvinBot]

Triggered auto assignment to @adelekennedy (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[dylanexpensify]

Thanks Adele! Will pick this up when I come back from OOO!

[adelekennedy]

@mdneyazahmad do you mind applying to the role in upwork, I can't find you for some reason!

[mdneyazahmad]

Update, I will create a pr by tomorrow.

[mdneyazahmad]

Pr is ready, testing locally will create soon.

[marcaaron]

Unnecessary re-renders makes the app slower and can make the app behave unexpectedly so it's always best to limit the amount of rendering.

Did we use any kind of instrumentation here to determine this is the cause of apparent slowness? Which things specifically are slow? And can we repeat those things to see a measurable improvement with some specific manual test steps?

[Beamanator]

Did we use any kind of instrumentation here to determine this is the cause of apparent slowness?

This issue isn't set up to eliminate current existing slowness (a.k.a. we're not experiencing slowness related to user.loginList), we're wanting to prevent future slowness / prevent a situation where we have to deal with unnecessary rerendering when subscribing to user onyx key

This was suggested & agreed on in the N7 Account Settings doc

[dylanexpensify]

Any update here @parasharrajat?

[parasharrajat]

PR deployed on staging 2 days ago.

[dylanexpensify]

Nice, thanks!

[botify]

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

• https://github.com/Expensify/App/pull/8264

If no regressions arise, payment will be issued on 2022-04-12. :confetti_ball:

[dylanexpensify]

Payments sent, contracts ended, posting removed!