-
Notifications
You must be signed in to change notification settings - Fork 53
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: forceAlign should not use outdated onAlign callback #86
Conversation
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
=======================================
Coverage 96.89% 96.89%
=======================================
Files 3 3
Lines 129 129
Branches 31 31
=======================================
Hits 125 125
Misses 4 4
Continue to review full report at Codecov.
|
Test case. Pls? |
Hi @zombieJ, thanks for reacting to this. Sadly, the bug ant-design/ant-design#27018 which caused me to trace the problem down into this component is really hard to reproduce, I assume a race condition that only triggers in complex applications due to the browser's timing behaviour (requestAnimationFrame) changing. I do not know how to provide a test case in this standalone environment here, or I would have done it. |
jest can hack timer, and you can inject raf to replace with |
It looks like I was able to build a test case. I think raf is not the issue, since that was actually only relevant in producing the race condition through rc-trigger -- I built a test case just with the existing hacked timers. I made sure to comment what is causing the issue for me and why I'm testing it in the test case itself. Note that this is changing behaviour, and it could be argued that the behaviour from before my changes was the correct one. I think however it is faulty, because it is caused by an internal setTimeout that is mostly invisible to the user. I think the new behaviour is more in line with what I would expect as a user, and it fixes the problem that occurs in ant-design (linked above). |
|
Fixes #85 -- I believe.