-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
img onLoad does not always fire in react 18 #17306
Comments
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
From what I can tell it is still an issue with the latest (241c446) experimental, so I don't think it should be closed. |
I thought #23316 would fix this but I think I can still repro. https://codesandbox.io/s/laughing-chandrasekhar-17l6tk?file=/src/index.js Maybe @gnoff could have a look. |
@gaearon I can't seem to repro. what browser are you using? Are you getting the error state on every retry or just in rare cases? |
Maybe I misunderstood the repro. What is the expected behavior? If it works, can you check whether it was reliably broken with the old version? |
Nm, i can was able to modify the sandbox to repro in some rare cases. were you seeing the image at the same time you saw To repro I
With those changes I was able to get the image to show with the wrong message indicating the load event was missed here it is failing usually on 18rc0: https://codesandbox.io/s/holy-shadow-cf0q21-react18rc0-cf0q21 Unless you are seeing different behavior than I am I think this one is done |
Yes, that’s what I saw. Does this mean the flash does not constitute a bug? My brain is bit fried and I’m not sure how to interpret the sandbox. |
The flash of the other text is from the 'pending' state. The original issue was that the pending state never disappeared. The use case of the real app was showing a spinner while loading a profile image and profile data for a profile page. I wanted an all-or-nothing loading experience to avoid showing profile data without the profile image. The spinner never disappeared as I never got the load event to trigger a state change to hide the spinner. The |
Ohh ok. So we can close this? |
I think so, but I'm not an expert |
@gaearon yesh I think this one is good. The flash is fine but it’s broken if it stays visible and that should be taken care of in 18 |
Do you want to request a feature or report a bug?
Bug
What is the current behavior?
<img onLoad={fn} />
does not always triggerfn
when rendered using createRoot if it has siblings which are heavy to render (I think?).If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
https://codesandbox.io/s/quiet-dawn-t1std
You might have to use the retry button a few times, but hopefully you should be able to see it. Don't disable cache while trying
What is the expected behavior?
onLoad always fires if img gets loaded.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
0.0.0-experimental-f6b8d31a7
Reproduced in chrome and firefox windows. Have not tried the codesandbox in mac, but the actual bug in our app occurred there first so I'd be surprised if it is OS specific.
It does not happen with regular "sync" mode.
The text was updated successfully, but these errors were encountered: