[HOLD for payment 2024-03-07] [$125] Web - Thread - Console error shows up when opening message in thread

[lanitochka17]

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

---

Version Number: 1.4.44-0
Reproducible in staging?: Y
Reproducible in production?: N
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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go to any chat
3. Send a message
4. Right click on the message > Reply in thread

Expected Result:

No console error will show up

Actual Result:

Console error shows up when opening message in thread

Workaround:

Unknown

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

Add any screenshot/video evidence

Bug6393509_1708999870296.bandicam_2024-02-27_06-35-39-042.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fd70450a1c9c7c93
• Upwork Job ID: 1762304222689734656
• Last Price Increase: 2024-02-27
• Automatic offers:
• dukenv0307 | Contributor | 0

[melvin-bot]

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

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @techievivek (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[askavyblr]

Proposal

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

Console error shows up when Reply in the thread

What is the root cause of that problem?

scrollRef.current?.getScrollableNode().childNodes[0] is undefined.

App/src/components/FlatList/MVCPFlatList.js

Line 49 in d2a9c6f

const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode().childNodes[0], []);

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

getScrollableNode() method returns undefined, meaning that there is no scrollable node available at the time this function is called.

Ensure that getScrollableNode() returns a valid element before trying to access its childNodes property.

const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode().childNodes[0], []);

Change To

const getContentView = React.useCallback(() => {
    const scrollableNode = scrollRef.current?.getScrollableNode();
    if (scrollableNode) {
        return scrollableNode.childNodes[0];
    }
    return null; // or handle the case when the scrollable node is not available
}, [scrollRef]);

What alternative solutions did you explore? (Optional)

[sobitneupane]

Thanks for the proposal @askavyblr.

I think we can simply do the following change to solve the issue.

-    const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode().childNodes[0], []);
+    const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode()?.childNodes[0], []);

cc: @techievivek

[melvin-bot]

Upwork job price has been updated to $125

[techievivek]

@sobitneupane Changes looks good to me, will you push the PR for this?

[techievivek]

I can review and merge.

[askavyblr]

@techievivek can i raise the PR

[sobitneupane]

Changes looks good to me, will you push the PR for this?

Sure @techievivek.

[dukenv0307]

@sobitneupane I think the following fix better addresses the root cause and avoiding running expensive effects, please consider that as well

Proposal

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

Console error shows up when opening message in thread

What is the root cause of that problem?

In here, we're triggering prepareForMaintainVisibleContentPosition on first render (by using useEffect) without making sure that the flatlist was actually mounted on the screen, this leads to the getScrollableNode here returning undefined (because the flatlist was not rendered yet), causing the console error.

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

We should make sure the flatlist was already rendered first before running the effect here, when the list was already rendered, getScrollableNode will always be defined and there's no need for optional chaining that are less readable.

Define a ref here

const isListRenderedRef = React.useRef(false);

In here:

onLayout={(e) => {
    isListRenderedRef.current = true;
    props.onLayout?.(e);
}}

In here

if (!isListRenderedRef.current) {
    return;
}

What alternative solutions did you explore? (Optional)

NA

[askavyblr]

@techievivek I will give you the solution first, please assign it to me

[sobitneupane]

@techievivek

@dukenv0307's proposal deals with the root cause of the issue. I think we can go with his proposal.

[melvin-bot]

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

[dukenv0307]

@sobitneupane PR #37261 is ready to review

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

• [CP Staging] Fix/37253: Console log error when opening thread #37261

If no regressions arise, payment will be issued on 2024-03-07. 🎊

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

• @sobitneupane requires payment through NewDot Manual Requests
• @dukenv0307 requires payment automatic offer (Contributor)

[melvin-bot]

Issue is ready for payment but no BZ is assigned. @MitchExpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

[MitchExpensify]

Payment summary:

C+: $125 @sobitneupane (NewDot)
C: $125 @dukenv0307 (Upwork)

I couldn't find any older contract for you @dukenv0307 so I created this job, let me know when you've accepted the offer and I'll issue payment 🙇 https://www.upwork.com/jobs/~016662aefe6aa80518

[techievivek]

Not overdue, we are issuing the payments. CC @MitchExpensify

[dukenv0307]

@youssef-lr @techievivek Hi I see after this comment, you've downgraded the issue bounty to $125, I assume that's because the change suggested is tiny, just adding the ? to avoid undefined object access.

However that "quick change" is a workaround. The root cause and solution of this issue is actually different and more complex (see here), and that's what we went with to fix the issue (with urgency as it was a deploy blocker).

Could we honor the original bounty ($500) for the issue since it was not dead simple as initially suggested?

[MitchExpensify]

Waiting on @dukenv0307 to accept the offer in order to pay 🙇

[youssef-lr]

@dukenv0307 I think I lowered the bounty due to this being just a console error and it's not really affecting the user. Do you think the console error is a smoke for an actual bug in the UI that is noticeable by the user?

[MitchExpensify]

Bump on accepting this offer as a next step @dukenv0307 https://www.upwork.com/nx/wm/offer/101299289

[dukenv0307]

@dukenv0307 I think I lowered the bounty due to this being just a console error and it's not really affecting the user

Thanks for the explanation.

Bump on accepting this offer as a next step @dukenv0307 https://www.upwork.com/nx/wm/offer/101299289

@MitchExpensify I accepted, thank you!

[MitchExpensify]

Paid and contract ended 🙇

[sobitneupane]

Requested payment in newDot.

Payment Summary: #37253 (comment)

[JmillsExpensify]

$125 approved for @sobitneupane based on summary.