[$1000] Dev - Console log error displayed on clicking 'Select a workspace' dropdown value

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

Precondition: account should have only one active workspace

1. Create a workspace & make sure you have only one active workspace
2. Click on 'FAB' menu ion
3. Click on 'New room'
   4, Click on 'Workspace' dropdown menu
4. Click on 'Select a workspace' option

Expected Result:

There should be no console log error

Actual Result:

Console log error displayed [Cannot update a component (Form) while rendering a different component (SafeAreaInsetsContext.Consumer).]

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 / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: Dev
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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
Notes/Photos/Videos: Any additional supporting documentation

Screencast.from.2023-07-27.16-25-04.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690464572048079

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c6e4120bf877ce30
• Upwork Job ID: 1685980808192278528
2023-07-31
• Automatic offers:
• natnael-guchima | Reporter | 25857396

[melvin-bot]

Triggered auto assignment to @trjExpensify (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

[ygshbht]

Proposal

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

The issue at hand involves a console.log error that appears when the Select a workspace option in the Workspace dropdown menu is clicked. The expected outcome is that no console.log error should be displayed during this process.

What is the root cause of that problem?

Upon reviewing the console error and the associated code, the root cause of the problem appears to be tied to a state update operation that occurs during the rendering of Form component. More specifically, the function that iterates over inputRefs.current and conditionally calls setInputValues is invoked during rendering, which leads to this error. This operation attempts to delete a reference and update the state while the component is rendering, which violates the rules of Hooks in React.

App/src/components/Form.js

Lines 348 to 362 in eb02006

	_.each(inputRefs.current, (inputRef, inputID) => {
	if (inputRef) {
	return;
	}
	delete inputRefs.current[inputID];
	setInputValues((prevState) => {
	const copyPrevState = _.clone(prevState);
	delete copyPrevState[inputID];
	return copyPrevState;
	});
	});

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

To resolve this issue, the state update operation needs to be moved out of the rendering process. We can accomplish this by wrapping the operation in a useEffect hook. This hook ensures that the state update operation is invoked as a side effect after the component render, rather than during the render itself.

Here is how we can modify the function:

useEffect(() => {
    _.each(inputRefs.current, (inputRef, inputID) => {
        if (inputRef) {
            return;
        }

        setInputValues((prevState) => {
            const copyPrevState = _.clone(prevState);

            delete copyPrevState[inputID];

            return copyPrevState;
        });
    });
}, [/* same dependencies as the childrenWrapperWithProps function */]);

By making this modification, we should be able to eliminate the console.log error.

2.New.Expensify.-.Google.Chrome.2023-07-30.18-09-09.mp4

_{Additionally, it may be possible to slightly optimize this code by moving the setInputValues function call outside of the _.each loop. However, this would require further investigation to ensure it doesn't introduce new issues or unexpected behavior.}

[s-alves10]

Proposal

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

Console error displayed When select Select a workspace in New Room page

What is the root cause of that problem?

When we select Select a workspace, Who can post text input disappears and the following code is run

        setInputValues((prevState) => {
            const copyPrevState = _.clone(prevState);

            delete copyPrevState[inputID];

            return copyPrevState;
        });

This code is called during the render process. Setting state during the rendering should be avoided and leads to an unexpected result. In addition, calling the set function of another component during rendering is an error as described here

This is the root cause

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

In order to avoid this pattern, we can use useEffect. useEffect is the perfect location we can change state
Move the above code into the useEffect

    useEffect(() => {
        _.each(inputRefs.current, (inputRef, inputID) => {
            if (inputRef) {
                return;
            }

            delete inputRefs.current[inputID];

            setInputValues((prevState) => {
                const copyPrevState = _.clone(prevState);

                delete copyPrevState[inputID];

                return copyPrevState;
            });
        });

    }, [childrenWrapperWithProps]);

This works great

What alternative solutions did you explore? (Optional)

[trjExpensify]

Moving on external for input from C+ as a dev bug report.

[melvin-bot]

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

[melvin-bot]

Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[rezkiy37]

Hi, I’m Michael (Mykhailo) from Callstack and I would like to work in this issue, because I am the author of the #21950.

[melvin-bot]

📣 @rushatgabhane Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot]

📣 @Natnael-Guchima 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[Natnael-Guchima]

Accepted the offer. Thanks.

[ygshbht]

Hi, so a call stack member can propose anytime and they'll get the job? @trjExpensify @rushatgabhane

[trjExpensify]

My understanding is that it's specifically addressing a regression from the PR author that introduced it, and so it makes sense to "fix your work" in this case.

[ygshbht]

So is he not going to get paid?

[trjExpensify]

I don't think Expensify's financial arrangement with agencies we partner with are in scope of the discussion we're having here, Yogesh.

[ygshbht]

Oh... makes sense. Apologies for the misunderstanding earlier

[rezkiy37]

#23919 was merged. Is the issue going to be closed?

[rushatgabhane]

created a manual request - https://staging.new.expensify.com/r/2106109152629952

[rushatgabhane]

this issue can be closed i guess

[trjExpensify]

We need to pay @Natnael-Guchima for the bug report right?

[rushatgabhane]

We need to pay @Natnael-Guchima for the bug report right

Yes!

[trjExpensify]

Okay, confirming payments:

• $250 to @Natnael-Guchima for the bug report (paid)
• $1000 to @rushatgabhane for the c+ review (he wasn't the reviewer of the PR that caused the regression)

Closing this out!

[JmillsExpensify]

Reviewed the details for @rushatgabhane. Approved for $1k payment in NewDot based on the BZ confirmation summary above.