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

fix: round preview coordinates to avoid infinity updates #253

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeetiss
Copy link

@jeetiss jeetiss commented Jan 8, 2024

preview is throw error Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops when dragged object has float coordinates or dimensions

rounding solves this problem

Copy link

sonarcloud bot commented Jan 8, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@LouisBrunner
Copy link
Owner

Hi @jeetiss,

I have seen this error many times for many different reasons. Do you have some test case that fails you can share with me?

@jeetiss
Copy link
Author

jeetiss commented Jan 9, 2024

Hi @LouisBrunner!

it is so hard to reproduce this error, i have logs for my previous reproduction,
i log updates from useCollector and there are results:

Снимок экрана 2024-01-09 в 12 47 23

preview works fine and then for some reason it going mad and swaps x from 8.749969482421875 to 8.750030517578125 50 times and this is cause Maximum update depth exceeded.

Rounding is the lightest fix, do you think it should be fixed in some other way?

@coveralls
Copy link

Coverage Status

coverage: 98.498%. remained the same
when pulling bdb1c6e on jeetiss:fix-infinity-updates
into 57ce33f on LouisBrunner:main.

@LouisBrunner
Copy link
Owner

it is so hard to reproduce this error, i have logs for my previous reproduction, i log updates from useCollector and there are results:

Could you give me an example on how you use useCollector?

preview works fine and then for some reason it going mad and swaps x from 8.749969482421875 to 8.750030517578125 50 times and this is cause Maximum update depth exceeded.

When I test it on my side, it never jumps unless the mouse move, which is why I am confused. This issue used to happen a lot in the past but the current implementation of calculatePointerPosition should be quite stable.

Rounding is the lightest fix, do you think it should be fixed in some other way?

This is a pretty error-prone part of the codebase. It's been responsible for many bugs in the past so I would like to understand what's going on before patching anything. For example, if the value is jumping back and forth then it's probably not due to a floating-point error and thus it could also happen on whole number (e.g. oscillating between 81 and 82) in which case rounding wouldn't fix it.

@Fun005
Copy link

Fun005 commented Jan 11, 2024

I have the same problem when I use uesPreview & <Mypreview>。I find if i use preview.ref,and drag some times, preview will throw error 。If I delete ref, error won`t happen.
image

@Fun005
Copy link

Fun005 commented Jan 11, 2024

If I use ref={ref.current}, no error, but ref won`t work
image

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.

None yet

4 participants