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

img onLoad does not always fire in react 18 #17306

Closed
peternycander opened this issue Nov 7, 2019 · 11 comments
Closed

img onLoad does not always fire in react 18 #17306

peternycander opened this issue Nov 7, 2019 · 11 comments
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Needs Investigation

Comments

@peternycander
Copy link

peternycander commented Nov 7, 2019

Do you want to request a feature or report a bug?
Bug
What is the current behavior?

<img onLoad={fn} /> does not always trigger fn 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.

@stale
Copy link

stale bot commented Feb 5, 2020

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.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Feb 5, 2020
@peternycander
Copy link
Author

From what I can tell it is still an issue with the latest (241c446) experimental, so I don't think it should be closed.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Feb 6, 2020
@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2022

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 gaearon added the React 18 Bug reports, questions, and general feedback about React 18 label Mar 30, 2022
@gaearon gaearon changed the title img onLoad does not always fire in concurrent mode img onLoad does not always fire in react 18 Mar 30, 2022
@gnoff
Copy link
Collaborator

gnoff commented Mar 30, 2022

@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?

@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2022

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?

@gnoff
Copy link
Collaborator

gnoff commented Mar 30, 2022

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 If image is visible, this seems like a bug? the default sandbox always flipped to All Good if the image was present, showing only a flash of the other text.

To repro I

  1. wrapped the render in startTransition
  2. added a bunch more divs
  3. replaced the src with the static asset src so there was no 302 redirect
  4. did a full "refresh", the retry button press would always settle to "All Good"

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
here it is working on 18: https://codesandbox.io/s/holy-shadow-cf0q21-react18-1ri1xp

Unless you are seeing different behavior than I am I think this one is done

@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2022

showing only a flash of the other text.

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.

@peternycander
Copy link
Author

peternycander commented Mar 31, 2022

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 If image is visible, this seems like a bug is the spinner.

@gaearon
Copy link
Collaborator

gaearon commented Mar 31, 2022

Ohh ok. So we can close this?

@peternycander
Copy link
Author

Ohh ok. So we can close this?

I think so, but I'm not an expert

@gaearon gaearon closed this as completed Mar 31, 2022
@gnoff
Copy link
Collaborator

gnoff commented Mar 31, 2022

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Needs Investigation
Projects
None yet
Development

No branches or pull requests

4 participants