[HOLD for payment 2024-01-24] [$500] AuthScreens reconnectApp call is passing an outdated lastUpdateIDAppliedToClient

[iwiznia]

See conversation in https://expensify.slack.com/archives/C01GTK53T8Q/p1701890693827559

Steps:

• Add a comment, I can see the lastUpdateID is returned and saved in the key OnyxUpdatesLastUpdateIDAppliedToClient
• Go offline and back online
• AuthScreens reconnect code (

  App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

  Lines 156 to 162 in d6c824b

  	NetworkConnection.onReconnect(() => {
  	if (isLoadingApp) {
  	App.openApp();
  	} else {
  	App.reconnectApp(lastUpdateIDAppliedToClient);
  	}
  	});

  ) runs
• But lastUpdateIDAppliedToClient is NOT the updated updateID we saved above

It is probably because this useEffect call

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 141 in d6c824b

useEffect(() => {

is passing [] as its dependency.
I was going to add lastUpdateIDAppliedToClient as a dependency but:

• I noticed there's a lot of other variables that we are using that are not passed to useEffect
• Not sure if there is a performance concern if I do that

I see that this behavior was introduced in this PR #27764 by @barttom, so ideally they would fix it.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018cbdee1256058e0a
• Upwork Job ID: 1732503147695874048
• Last Price Increase: 2023-12-06
• Automatic offers:
• jjcoffee | Reviewer | 27989002
• situchan | Contributor | 27989059

[melvin-bot]

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[aldo-expensify]

@barttom I'm proposing the following fix: https://github.com/Expensify/App/pull/32585/files
Leaving it there in case you find it useful

It is consistent with how we are doing it for isLoadingApp

[situchan]

Is there any bug I can reproduce in the app because of this issue? Or just dev fix?

[iwiznia]

I don't get the question, the bug is described above. We should be passing the latest updateID, but instead we are passing an old one, so we are getting updates we don't need and those updates will be bigger and bigger over time (assuming you don't reload the page or restart the app)

[situchan]

I mean for production user perspective who doesn't know app code

[aldo-expensify]

I mean for production user perspective who doesn't know app code

Can lead to performance issues, the backend fetches and sends more data than it should

[iwiznia]

I mean for production user perspective who doesn't know app code

No, not really.

[barttom]

Hey!
I can look into this on Friday or next week.
However, I don't think that my PR created this issue.
Before my PR, this callback was created in the componentDidMount event, so the first value of lastUpdateIDAppliedToClient has been cached.
The whole logic has been restored with hooks, some in that case hook useEffect with empty dependencies to run the callback once on componentDidMount event. Adding lastUpdateIDAppliedToClient to the dependencies will launch all logic from useEffect callback for every change of this dependency/

[iwiznia]

Check the conversation, we've "dismissed" what that last comment is saying.