mbrevda / react-image

React.js <img> tag rendering with multiple fallback & loader support
MIT License
1.2k stars 88 forks source link

App freeze when using React 18 #952

Closed Myrfion closed 1 year ago

Myrfion commented 2 years ago

Hello, I upgraded the app to react 18 from reacting 17 and in the places of the app where I use react-image, it gets freezer and stops responding (feels like it gets in some infinite loop). Has anybody else faced something like that?

anthonykrivonos commented 2 years ago

@Myrfion Was losing my mind the past hour trying to figure out what was happening, looks like I'm not alone 🥵

Myrfion commented 2 years ago

@anthonykrivonos I was losing my mind for almost a day, cuz I wasn't able to believe that nobody faces that issue with the package that has 50k downloads weekly...

mbrevda commented 2 years ago

Thanks guys for reporting. I haven't moved any of my phone projects to React 18 yet, but will prioritize looking in to this.

mbrevda commented 2 years ago

I set up a branch that, among other things, moves to react 18 but am not seeing this. Did I miss something?

Can you please link to a codesandbox illustrating the issue? Alternatively, try cloning that branch, run npm run dev, and visit http://localhost:3888/ to run the test suit.

mgoodfellow commented 2 years ago

Also suffering the same issue, but haven't managed to build a small reproducible sample yet. Seems to occur on re-renders of the tree which it is a part of

The infinite loop it gets stuck in is in react-dom:

function basicStateReducer(state, action) {
  // $FlowFixMe: Flow doesn't like mixed types
  return typeof action === 'function' ? action(state) : action;
}
shardbearer-shri commented 2 years ago

Is there any other alternate solution for this ? All i wanted to do was render a fallback element on image error :(

mgoodfellow commented 2 years ago

Well, I made a quick component which is drop in compatible just for fallback images when src is an array, and I always know it will have a length of 2 - it's hacky but we really just needed to quickly unblock ourselves and also prove that it was only react-image which was causing issues (in our case that was the case)

import isArray from 'lodash/isArray';
import React, { useEffect, useRef, useState } from 'react';

type ImgProps = {
    src: string | string[];
    alt?: string;
    style?: any;
    srcSet?: string;
};

const HtmlImgFallback = ({ src, ...rest }: ImgProps) => {
    const [imgSrc, setImgSrc] = useState<string>(isArray(src) ? src[0] : src);
    const imgRef = useRef<HTMLImageElement>();

    useEffect(() => {
        setImgSrc(isArray(src) ? src[0] : src);
    }, [src]);

    return (
        <img
            {...rest}
            ref={imgRef}
            src={imgSrc}
            onLoad={() => {
                if (imgRef.current?.clientWidth === 0) {
                    // Broken image
                    setImgSrc(isArray(src) ? src[1] : src);
                }
            }}
            onError={() => {
                setImgSrc(isArray(src) ? src[1] : src);
            }}
        />
    );
};

export default HtmlImgFallback;

This doesn't fit your use case, but I'm sure you can quickly adapt it to render out a fallback component you passed in on error.

Having swapped our usage out we haven't seen any more issues - we weren't using the advanced features of react-image, so it made sense for us to use this as a stop gap as we have been forced to upgrade to React 18.

martdavidson commented 2 years ago

We're seeing this as well, but only when useSuspense: false which we need.

Ah, but even without disable suspense, it console.errors:

Can't perform a React state update on a component that hasn't mounted yet. This indicates that you have a side-effect in your render function that asynchronously later calls tries to update the component. Move this work to useEffect instead.

clueking1 commented 2 years ago

My app freezes when using the onClick prop and then clicking the image

idChef commented 2 years ago

@mbrevda I have created reproduction of the issue on CodeSandbox

mbrevda commented 2 years ago

Oh wow, thanks! That is sooo strange. I'll try to dig into this a bit more, open to ideas!

idChef commented 2 years ago

I also can't get the useImage approach to work Codesandbox reproduction

In my app that I'm trying to implement react-image in I have this error: image

mbrevda commented 2 years ago

update: not 100% clear about what changed in React 18, but seems returning earlier in the hook resolves the issue. Please note, PR still requires lots of work, so the fix is not ready yet.

RykerMunn commented 2 years ago

@mbrevda do you have an estimated timeline for when this fix might be ready? My team has had to come up with a workaround for this problem for one of the key features in our software, which is working now but we are experiencing the same freezing problem as detailed above.

Specifically in our case we change the src passed to the Img component based on the props passed into our viewer. We make an API request to get a different sized image based on what the user's image preference is. When they change the size, we generate a new source URL and update the Img component.

This CodeSandbox shows approximately what our setup is. Although it doesn't appear to actually freeze in this particular case.

Titan-Codes commented 1 year ago

@mbrevda is this issue fixed yet?

10thfloor commented 1 year ago

We're experiencing this as well, after upgrading to React 18. Has this been fixed?

mbrevda commented 1 year ago

Hi all -

First of all, apologies for the delay in addressing this.

Second, I've published a canary build addressing the issue. I'd appreciate if you could take a moment to confirm that this build fixes the issue for you before I publish. You can install it with npm i react-image@0.0.0-c47ea58a7c281cbab29f3272c0a4d4c58dc2aaa7 and re-trying whatever was causing the freeze.

You can also see this working in an updated codesandbox

mbrevda commented 1 year ago

Was anyone able to test? Would like to release this...

RykerMunn commented 1 year ago

@mbrevda sorry for the delay in getting back to you. My team has deployed version 4.1.0 to our dev environment and it has solved this app freezing in our use case.

Thank you for publishing a fix!

mbrevda commented 1 year ago

Awesome, so glad to hear!