-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add safe default fallback for scrollEventThrottle and remove return from queueMicrotask #4777
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
Conversation
| // to have continuous scroll events and | ||
| // react-native defaults it to 50 for FlatLists. | ||
| // We set it to 1 so we have peace until | ||
| // there are 960FPS screens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // there are 960FPS screens. | |
| // there are 960 fps screens. |
| } else { | ||
| exportedModule = new NativeReanimated(); | ||
| } | ||
| const NativeReanimatedModule = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use exportedModule as for now.
| // Set default scrollEventThrottle, because user expects | ||
| // to have continuous scroll events. | ||
| // We set it to 1 so we have peace until | ||
| // there are 960FPS screens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // there are 960FPS screens. | |
| // there are 960 fps screens. |
src/reanimated2/threads.ts
Outdated
| } | ||
|
|
||
| export const callMicrotasks = shouldBeUseWeb() | ||
| export const callMicrotasks = IS_WEB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at line #9 😎
tomekzaw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approvele
Summary
queueMicrotaskthat remained there when it was changed from setImmediate.scrollEventThrottleinAnimated.ScrollViewandAnimated.FlatList.We go from
to
Because if user deliberately sets
scrollEventThrottleto 0 we would override it.Test plan
Test code for scrollEventThrottle, just fiddle a bit with this prop and see the outcome in the console.