Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition between hydration and image load event #629

Closed
Stouffi opened this issue Jan 7, 2022 · 10 comments · Fixed by #630
Closed

Race condition between hydration and image load event #629

Stouffi opened this issue Jan 7, 2022 · 10 comments · Fixed by #630

Comments

@Stouffi
Copy link
Contributor

Stouffi commented Jan 7, 2022

What package has an issue

@mantine/core

Describe the bug

When an Image with placeholder is server-side rendered, the component displayed in browser can be hydrated after load event and in this case will keep displaying placeholder.

In which browser did the problem occur

Any

If possible, please include a link to a codesandbox with the reproduced problem

No response

Do you know how to fix the issue

Yes

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

Looking at this issue's comment facebook/react#15446 (comment)

I could implement a fix for Image with a useEffect

useEffect(() => {
  const { current } = internalImgRef;
  if (current?.complete) {
    setError(current.naturalHeight === 0);
    setLoaded(current.naturalHeight !== 0);
  }
}, [src])
@rtivital
Copy link
Member

rtivital commented Jan 7, 2022

Thanks for reporting, you can submit a PR with a fix

@rtivital
Copy link
Member

rtivital commented Jan 8, 2022

Released in 3.5.3

@Stouffi
Copy link
Contributor Author

Stouffi commented Feb 11, 2022

@rtivital please consider reopening this issue because of a577d24 rolling back #630

from 3.6.6 the race condition with hydration and load event can still make the placeholder visible.

Could you also explain why rolling back was necessary?

@rtivital
Copy link
Member

I had to roll back because Image stopped working with dynamic values and after hydration in Next.js, rolling back fixed the issue

@Yonben
Copy link

Yonben commented Feb 27, 2022

@rtivital Not sure I understood what you meant with your last answer. I updated to latest (3.6.11) and it's still happening to me.

I get placeholder permanently most of the time when server rendered. When navigating my app and having the client rendering it, works fine though.

@rtivital
Copy link
Member

@Yonben I'm saying that:

  • I cannot reproduce this issue with Next.js ssr
  • PR that was sent to fix this issue that I cannot reproduce broke images loading
  • I had to rollback PR changes in order to make it work in Next.js.

If you can provide an example repo that reproduces this issue we'll be able to debug.

@Yonben
Copy link

Yonben commented Feb 27, 2022

Will do my best and tag you if I manage to :) Thanks a lot!

@Yonben
Copy link

Yonben commented Feb 27, 2022

@rtivital Here it is.
https://codesandbox.io/s/focused-panka-ieeh66?file=/pages/avatar/%5Bname%5D.js
If on this page you go to /avatar/<arandomname> you will get the placeholder. However if then you use the input and click the button to change the route on the client side, it will work.

Thanks for the help 🙏

Edit: forgot to actually put the link...

@rtivital
Copy link
Member

Thanks for reproduction, I'll have a look

@rtivital rtivital reopened this Feb 27, 2022
@rtivital rtivital added the bug label Feb 27, 2022
@rtivital rtivital removed the bug label Aug 11, 2022
@rtivital
Copy link
Member

Fixed in 5.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants