[$500] [Dev] Infinite receipt loading spinner with backend dev environment

[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:
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
Expensify/Expensify Issue URL:
Issue reported by: @neil-marcellini
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1719847597306219

Please dm @neil-marcellini on Slack during PDT hours if you would like to test against my dev env.

Action Performed:

1. Open New Expensify running on a backend dev environment (ask an internal engineer to share theirs via ngrok)
2. View an expense with a receipt

Expected Result:

Once the receipt is loaded there won't be a loading spinner

Actual Result:

Once the receipt is loaded there won't be a loading spinner

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

Screen.Recording.2024-07-01.at.8.21.57.AM.mov

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014e28bd547489ae41
• Upwork Job ID: 1807839133623941808
• Last Price Increase: 2024-07-25

[melvin-bot]

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

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014e28bd547489ae41

[melvin-bot]

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

[Taiwrash]

Proposal

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

• The spinner keep rolling after the receipt as loaded and currently not showing the image again

What is the root cause of that problem?

• Width value of the image is undefined and cannot be determined
  App/blob/main/src/components/Image/BaseImage.tsx#L7-L18

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

• Handle the state using the callback and memo correctly by updating the imageLoadedSuccessfully function with

const imageLoadedSuccessfully = useCallback(
    (event: {nativeEvent: ImageLoadEventData}) => {
        if (!onLoad) {
            return;
        }

        // We override `onLoad`, so both web and native have the same signature
        const source = event.nativeEvent.source;
        if (source && typeof source === 'object') {
            const {width, height} = source;
            onLoad({nativeEvent: {width, height}});
        } else {
            // Handle the case where source is undefined or not an object
            console.warn('Image source is undefined or not an object');
            onLoad({nativeEvent: {width: 0, height: 0}}); 
        }
    },
    [onLoad],
);

What alternative solutions did you explore? (Optional)

[ishpaul777]

This might be dupe of #44566, I can also reproduce with stag/prod server, setting USE_REACT_STRICT_MODE=false in CONFIG.js should resolve the issue temporarily.

[Taiwrash]

@ishpaul777 yes, it looks the same but I think for this demo, the app is not crashing which is different from the previous suggestion

[eh2077]

@Taiwrash Can you please elaborate on the root cause of this issue?

[Taiwrash]

YES!

The main cause of the error is the improper cleaning up of the data after fetching it from the backend. Although this can also be handled by setting the right dependencies for the useEffect where data fetching is happening. In summary, the root cause is the improper way of state update and wrong cleanup

[neil-marcellini]

@Taiwrash thanks for elaborating. Would you please update your original proposal with links to the current code causing the root problem and explain exactly what the issue is, along with your proposed solution? Please see our contributing guidelines and you can also look at proposals on other issue to get an idea of how they are written.

Thanks @ishpaul777 for recommending setting USE_STRICT_MODE = false. I'll try that.

[neil-marcellini]

This might be dupe of #44566,

Let's keep an eye on that one because it might have the same fix or root cause, but this feels a bit different so I'll keep it separate for now.

[neil-marcellini]

Setting USE_REACT_STRICT_MODE: false in Config.ts doesn't seem to solve the problem.

[ishpaul777]

Sorry i take my words back its reproducable but USE_REACT_STRICT_MODE: false does solve it for me

Screen.Recording.2024-07-04.at.12.10.16.AM.mov

[melvin-bot]

@neil-marcellini, @kadiealexander, @eh2077 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[eh2077]

Still looking for proposals

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@neil-marcellini does this still happen for you?

[melvin-bot]

Upwork job price has been updated to $500

[Taiwrash]

Yes, and now the receipt images don't load at all. Let's double the price.

Do you have a new error when the receipt is not showing now?

Kindly help with screenshot or video of the current state. I am sorry i have been out on this issue before now

[Taiwrash]

I figured out the error is due to the failure to receive width properties from from the event.nativeEvent in some cases of the responses

const {width, height} = event.nativeEvent.source;

Here is a quick hack I came up to fix the issue for now

const imageLoadedSuccessfully = useCallback(
    (event: {nativeEvent: ImageLoadEventData}) => {
        if (!onLoad) {
            return;
        }

        // We override `onLoad`, so both web and native have the same signature
        const source = event.nativeEvent.source;
        if (source && typeof source === 'object') {
            const {width, height} = source;
            onLoad({nativeEvent: {width, height}});
        } else {
            // Handle the case where source is undefined or not an object
            console.warn('Image source is undefined or not an object');
            onLoad({nativeEvent: {width: 0, height: 0}}); 
        }
    },
    [onLoad],
);

• The hacks: check source availability and is an object before destructuring.

[eh2077]

@Taiwrash Thanks for sharing your findings. Can you please update your proposal based on suggestion here #44692 (comment)

[Taiwrash]

Proposal

Updated

[Taiwrash]

@neil-marcellini @eh2077 looking forward to your review and besides i haven't been added to the slack channel, all the links i am seeing are not working for me. they are requesting for a unique email address

[eh2077]

@Taiwrash

Width value of the image is undefined and cannot be determined

Why does this keep images loading?

[Taiwrash]

@Taiwrash

Width value of the image is undefined and cannot be determined

Why does this keep images loading?

1. The code is trying to access the width property of an object that is undefined. This is happening in the image loading callback
2. The code is structure to use the loading spinner while waiting for images to load, the spinner's behaviour is tied to the successful completion of the image loading process which is not completing.
3. The error has prevented the normal flow of the code that would otherwise signal that the image has finished loading, in other words, code to update the state of the spinner is not executed

Those are my observations @eh2077

[dominictb]

Proposal

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

The receipt is infinitely loading in dev mode. In my machine it's the behavior is different from what I see in the evidence video: the image doesn't load at all. However, I believe all share the same root cause.

What is the root cause of that problem?

When enabling Strict Mode, React 18 simulates unmounting/mounting the component. Read more here. However, it seems like it doesn't re-create the ref during that simulation, and we'll have trouble with our patch for react-native-web's Image component.

During the unmount simulation, the image request will be cancelled here. However, since the ref is not re-created, in the next mount simulation this condition will hold true as the props.source is the same, hence, the
image will never be loaded. We can easily check the Network tab and see that the image request gets cancelled.

This also explains if we disable the strict mode, as there's no mount/unmount simulation, the request is never cancelled and we got the image loaded successfully.

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

The idea is that if the request got cancelled during unmount/mount simulation, we should catch that and allows the request to be re-created during in the next mount. Details are below:

In here, introduce isCancelled function

isCancelled: () => abortController.signal.aborted

By default, in here, isCancelled() should always be true

+  var request = React.useRef({
+    cancel: () => {},
+    source: {
+      uri: '',
+      headers: {}
+    },
+    promise: Promise.resolve(''),
+.   isCancelled: () => true,
+  });

And in here, we should allow the request ref to be re-created (i.e: start a new image request) in case the current request is cancelled

if (!hasSourceDiff(nextSource, request.current.source) && !request.current.isCancelled()) {
   return;
}

What alternative solutions did you explore? (Optional)

N/A

[eh2077]

@dominictb Thank you for sharing your investigation!

[eh2077]

This might be dupe of #44566,

Let's keep an eye on that one because it might have the same fix or root cause, but this feels a bit different so I'll keep it separate for now.

@neil-marcellini I think we should put this on hold for #44566 as they're very likely to share the same root cause.

Setting USE_REACT_STRICT_MODE: false in Config.ts doesn't seem to solve the problem.

Now it solved the problem if USE_REACT_STRICT_MODE: false. It also didn’t work in my previous testing.

[neil-marcellini]

This can be take off hold now. USE_REACT_STRICT_MODE: false does indeed fix the problem now, but it would be great to get this working with strict mode enabled too. @eh2077 @dominictb will your proposal work as is or do you need to make any updates now that the issue is off hold?

[eh2077]

There're some recent updates on patches/react-native-web+0.19.12+003+image-header-support.patch from PR https://github.com/Expensify/App/pull/45559/files. I think we need to diagnose the root cause again.

@dominictb Would you like to take a look into this?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[dominictb]

@eh2077 I think we can close this issue. That patch itself fix this problem as well. Thanks!

[neil-marcellini]

Ah yeah, the problem is fixed after I double checked. Sweet!

I had to run this to get the package patch to apply properly.

npm cache clean --force
rm -rf node_modules
npm install