[HOLD for payment - waiting on deploy][$250] Opening Two-Factor Authentication settings causes two duplicate API requests for EnableTwoFactorAuth

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.24-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @tgolen
Slack conversation: https://expensify.slack.com/archives/C05LX9D6E07/p1724704755405909

Action Performed:

1. Go to staging.new.expensify.com
2. Click Account settings > Security > Two Factor authentication setting

Expected Result:

No duplicate calls API calls

Actual Result:

Two duplicate API requests for EnableTwoFactorAuth

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

2024-08-26_14-37-41.mp4

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0184daf84295192016
• Upwork Job ID: 1828808342978232742
• Last Price Increase: 2024-09-04
• Automatic offers:
• Nodebrute | Contributor | 103846483

[melvin-bot]

Current assignee @tgolen is eligible for the AutoAssignerNewDotQuality assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-08-28 16:13:58 UTC.

Proposal

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

Opening Two-Factor Authentication settings causes two duplicate API requests for EnableTwoFactorAuth

What is the root cause of that problem?

The behavior you're seeing, where not just the two-factor authentication call but all calls in useEffect are running twice, is due to React's Strict Mode. This change was introduced in this PR and is only in the development environment. We do not see this behavior in the production build.

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

I prefer turning off Strict Mode, as introduced in this PR This way, development and production environments will behave the same.

Note: StrictMode is not needed for concurrent react to work but it is much safer to use it.

We can disable strict mode from here

App/src/CONFIG.ts

Line 100 in cadd5a8

USE_REACT_STRICT_MODE_IN_DEV: true,

What alternative solutions did you explore? (Optional)

We can create a ref and then in useEffect we can early return if it's already mounted. With the implementation of useRef, we ensure that API calls within useEffect are made only once, even in development mode with React's Strict Mode enabled. This method prevents multiple invocations of the effect due to Strict Mode's behavior.

App/src/pages/settings/Security/TwoFactorAuth/Steps/CodesStep.tsx

Line 46 in a6564ac

useEffect(() => {

We can do something like this

    const isMounted = useRef(false);
    useEffect(() => {
        if (isMounted.current) {
            return; // Exit early if effect has already run
        }
        // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
        if (account?.requiresTwoFactorAuth || account?.recoveryCodes || !isUserValidated) {
            return;
        }
        Session.toggleTwoFactorAuth(true);
        isMounted.current = true;
     
    }, [isUserValidated]);

[abzokhattab]

Not able to reproduce the issue on prod

[tgolen]

Thanks for the investigation @Nodebrute. I think the thing that I really dislike about that is now dev is a very different environment than production, which makes testing and development much more difficult. We are being forced into some weird patterns (using a bunch of useRef) to try and get development to work more like we expect production to work.

Do you know if React is planning to make production work in this concurrent mode like dev is running in? I guess that would return it to an apples-to-apples situation at some point, which is better, but I really don't like having to guess "does this only happen in dev or not".

I'm going to assign this to external and get a C+ to review your proposal to implement (because even though it only happens in dev, it messed me up enough that I think it's worth fixing).

[melvin-bot]

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