[HOLD for payment 2023-07-21] [Performance] Reduce re-renders of ReportScreen when re-opening/switching it

[hannojg]

What performance issue do we need to solve?

We want to optimise the performance when switching the chat/report screen.

What is the impact of this on end-users?

Improved TTI when opening another report.

List any benchmarks that show the severity of the issue

When using the react-devtools profiler we can track the commits react does. When opening a report screen, we can see that there is this is, after the ReportScreen has already been rendered another render. This render is due to the fact that a state variable called skeletonViewContainerHeight got changed:

In the current code-design we do need to measure that height once, however, this doesn't need to cause a re-render when we are switching the chat screen.

Proposed solution (if any)

We are already storing the skeleton height globally to reuse it, all we need to do is stop the setState call.

Here, we store and reuse the height value:

App/src/pages/home/ReportScreen.js

Lines 120 to 134 in ecfcd83

	// Keep a reference to the list view height so we can use it when a new ReportScreen component mounts
	let reportActionsListViewHeight = 0;
	class ReportScreen extends React.Component {
	gestureStartListener = null;
	constructor(props) {
	super(props);
	this.onSubmitComment = this.onSubmitComment.bind(this);
	this.chatWithAccountManager = this.chatWithAccountManager.bind(this);
	this.dismissBanner = this.dismissBanner.bind(this);
	this.state = {
	skeletonViewContainerHeight: reportActionsListViewHeight,

Now, all we need to do is to stop the setState call here, which could look like this:

<View
    nativeID={CONST.REPORT.DROP_NATIVE_ID + this.getNavigationKey()}
    style={[styles.flex1, styles.justifyContentEnd, styles.overflowHidden]}
    onLayout={(event) => {
        // Rounding this value for comparison because they can look like this: 411.9999694824219
        const skeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height);

        // ⚠️👀 PROPOSAL: Only set state when the height changes to avoid unnecessary renders
        if (reportActionsListViewHeight === skeletonViewContainerHeight) return;

        // The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it
        // takes up so we can set the skeleton view container height.
        if (skeletonViewContainerHeight === 0) {
            return;
        }
        reportActionsListViewHeight = skeletonViewContainerHeight;
        this.setState({skeletonViewContainerHeight});
    }}

(Note: another solution would be to turn the ReportScreen into a PureComponent, however, this if check is should be cheaper then a PureComponent)

List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)

When measuring with the profiler again, we can see that the additional render of the ReportScreen is gone (note: only true when opening the ReportScreen for the second time):

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: latest/main
Reproducible in staging?: yes
Reproducible in production?: yes
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on Upwork

[melvin-bot]

Triggered auto assignment to @MariaHCD (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MariaHCD]

I don't believe I need to be assigned to this :)

[melvin-bot]

@hannojg Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mountiny]

Handling the PR review so assigning to myself

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

• [Performance fix #21831] Don't re-render ReportScreen unnecessarily when switching #21832

If no regressions arise, payment will be issued on 2023-07-21. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty