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

app: wait one tick before setNewOriginalImage #334

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

pulsejet
Copy link
Contributor

This works around a bug in react (more details in inline comments).

Just want to make a note here that I've tested this extensively and it solves many unexpected issues (also took a very long time to debug this!)

This works around a bug in react (more details in inline comments)

Signed-off-by: Varun Patil <[email protected]>
@pulsejet
Copy link
Contributor Author

@AhmeeedMostafa

@AhmeeedMostafa
Copy link
Collaborator

@pulsejet could u please explain what the bug is exactly, as it is not clear

@pulsejet
Copy link
Contributor Author

@AhmeeedMostafa there's a bug in react apparently, that doesn't clean up some references during the first tick. Don't have time to fix react itself, unfortunately :(

@AhmeeedMostafa
Copy link
Collaborator

@AhmeeedMostafa there's a bug in react apparently, that doesn't clean up some references during the first tick. Don't have time to fix react itself, unfortunately :(

Could u please provide some example production steps about the issue, or attach the react issue's url?

@pulsejet
Copy link
Contributor Author

Here: https://codepen.io/pulsejet/pen/gOBQobe

Here's a video of the steps to reproduce below (in order): https://youtu.be/iUFmuV5YIG4

(reload the browser with cache disabled before each test).
Also since codepen uses iframe this doesn't work in incognito. Tested on latest Chrome/Firefox on Windows 11.

Test 1:
CREATE WITH LINK: Works fine.
Test 2:
CREATE WITH ELEM, then DESTROY, then CREATE WITH ELEM again => broken
Test 3:
CREATE WITH ELEM WITH DELAY, then DESTROY, then CREATE WITH ELEM WITH DELAY again => works now (this PR has the equivalent effect)

@AhmeeedMostafa

@AhmeeedMostafa AhmeeedMostafa changed the base branch from master to v4-dev June 5, 2023 16:58
@AhmeeedMostafa AhmeeedMostafa merged commit d37ce96 into scaleflex:v4-dev Jun 5, 2023
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 this pull request may close these issues.

2 participants