-
Notifications
You must be signed in to change notification settings - Fork 217
refactor(container,lazy,hooks): avoid blocking JS thread by refactoring SharedValue.value access #423
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
refactor(container,lazy,hooks): avoid blocking JS thread by refactoring SharedValue.value access #423
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,5 @@ | ||
| import React from 'react' | ||
| import { | ||
| LayoutChangeEvent, | ||
| StyleSheet, | ||
| useWindowDimensions, | ||
| View, | ||
| } from 'react-native' | ||
| import { StyleSheet, useWindowDimensions, View } from 'react-native' | ||
| import PagerView from 'react-native-pager-view' | ||
| import Animated, { | ||
| runOnJS, | ||
|
|
@@ -15,6 +10,7 @@ import Animated, { | |
| useSharedValue, | ||
| withDelay, | ||
| withTiming, | ||
| useFrameCallback, | ||
| } from 'react-native-reanimated' | ||
|
|
||
| import { Context, TabNameContext } from './Context' | ||
|
|
@@ -27,6 +23,7 @@ import { | |
| useContainerRef, | ||
| usePageScrollHandler, | ||
| useTabProps, | ||
| useLayoutHeight, | ||
| } from './hooks' | ||
| import { | ||
| CollapsibleProps, | ||
|
|
@@ -93,25 +90,29 @@ export const Container = React.memo( | |
| const windowWidth = useWindowDimensions().width | ||
| const width = customWidth ?? windowWidth | ||
|
|
||
| const containerHeight = useSharedValue<number | undefined>(undefined) | ||
| const [containerHeight, getContainerLayoutHeight] = useLayoutHeight() | ||
|
|
||
| const tabBarHeight = useSharedValue<number | undefined>( | ||
| initialTabBarHeight | ||
| ) | ||
| const [tabBarHeight, getTabBarHeight] = | ||
| useLayoutHeight(initialTabBarHeight) | ||
|
|
||
| const headerHeight = useSharedValue<number | undefined>( | ||
| const [headerHeight, getHeaderHeight] = useLayoutHeight( | ||
| !renderHeader ? 0 : initialHeaderHeight | ||
| ) | ||
| const initialIndex = React.useMemo( | ||
| () => | ||
| initialTabName | ||
| ? tabNamesArray.findIndex((n) => n === initialTabName) | ||
| : 0, | ||
| [initialTabName, tabNamesArray] | ||
| ) | ||
|
|
||
| const contentInset = useDerivedValue(() => { | ||
| const contentInset = React.useMemo(() => { | ||
| if (allowHeaderOverscroll) return 0 | ||
|
|
||
| // necessary for the refresh control on iOS to be positioned underneath the header | ||
| // this also adjusts the scroll bars to clamp underneath the header area | ||
| return IS_IOS | ||
| ? (headerHeight.value || 0) + (tabBarHeight.value || 0) | ||
| : 0 | ||
| }) | ||
| return IS_IOS ? (headerHeight || 0) + (tabBarHeight || 0) : 0 | ||
| }, [headerHeight, tabBarHeight, allowHeaderOverscroll]) | ||
|
|
||
| const snappingTo: ContextType['snappingTo'] = useSharedValue(0) | ||
| const offset: ContextType['offset'] = useSharedValue(0) | ||
|
|
@@ -131,22 +132,16 @@ export const Container = React.memo( | |
| () => tabNamesArray, | ||
| [tabNamesArray] | ||
| ) | ||
| const index: ContextType['index'] = useSharedValue( | ||
| initialTabName | ||
| ? tabNames.value.findIndex((n) => n === initialTabName) | ||
| : 0 | ||
| ) | ||
| const index: ContextType['index'] = useSharedValue(initialIndex) | ||
|
|
||
| const focusedTab: ContextType['focusedTab'] = | ||
| useDerivedValue<TabName>(() => { | ||
| return tabNames.value[index.value] | ||
| }, [tabNames]) | ||
| const calculateNextOffset = useSharedValue(index.value) | ||
| const calculateNextOffset = useSharedValue(initialIndex) | ||
| const headerScrollDistance: ContextType['headerScrollDistance'] = | ||
| useDerivedValue(() => { | ||
| return headerHeight.value !== undefined | ||
| ? headerHeight.value - minHeaderHeight | ||
| : 0 | ||
| return headerHeight !== undefined ? headerHeight - minHeaderHeight : 0 | ||
| }, [headerHeight, minHeaderHeight]) | ||
|
|
||
| const indexDecimal: ContextType['indexDecimal'] = useSharedValue( | ||
|
|
@@ -167,7 +162,7 @@ export const Container = React.memo( | |
| scrollToImpl( | ||
| refMap[name], | ||
| 0, | ||
| scrollYCurrent.value - contentInset.value, | ||
| scrollYCurrent.value - contentInset, | ||
| false | ||
| ) | ||
| } | ||
|
|
@@ -213,6 +208,33 @@ export const Container = React.memo( | |
| [onIndexChange, onTabChange] | ||
| ) | ||
|
|
||
| const syncCurrentTabScrollPosition = () => { | ||
| 'worklet' | ||
|
|
||
| const name = tabNamesArray[index.value] | ||
| scrollToImpl( | ||
| refMap[name], | ||
| 0, | ||
| scrollYCurrent.value - contentInset, | ||
| false | ||
| ) | ||
| } | ||
|
|
||
| /* | ||
| * We run syncCurrentTabScrollPosition in every frame after the index | ||
| * changes for about 1500ms because the Lists can be late to accept the | ||
| * scrollTo event we send. This fixes the issue of the scroll position | ||
| * jumping when the user changes tab. | ||
| * */ | ||
| const toggleSyncScrollFrame = (toggle: boolean) => | ||
| syncScrollFrame.setActive(toggle) | ||
| const syncScrollFrame = useFrameCallback(({ timeSinceFirstFrame }) => { | ||
| syncCurrentTabScrollPosition() | ||
| if (timeSinceFirstFrame > 1500) { | ||
| runOnJS(toggleSyncScrollFrame)(false) | ||
| } | ||
| }, false) | ||
|
|
||
| useAnimatedReaction( | ||
| () => { | ||
| return calculateNextOffset.value | ||
|
|
@@ -236,13 +258,14 @@ export const Container = React.memo( | |
| scrollYCurrent.value = | ||
| scrollY.value[tabNames.value[index.value]] || 0 | ||
| } | ||
| runOnJS(toggleSyncScrollFrame)(true) | ||
| } | ||
| }, | ||
| [] | ||
| ) | ||
|
|
||
| useAnimatedReaction( | ||
| () => headerHeight.value, | ||
| () => headerHeight, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this actually work right? Can reanimated trigger a reaction based on a react state change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, it does work, I added logs to validate. |
||
| (_current, prev) => { | ||
| if (prev === undefined) { | ||
| // sync scroll if we started with undefined header height | ||
|
|
@@ -267,32 +290,6 @@ export const Container = React.memo( | |
| } | ||
| }, [revealHeaderOnScroll]) | ||
|
|
||
| const getHeaderHeight = React.useCallback( | ||
| (event: LayoutChangeEvent) => { | ||
| const height = event.nativeEvent.layout.height | ||
| if (headerHeight.value !== height) { | ||
| headerHeight.value = height | ||
| } | ||
| }, | ||
| [headerHeight] | ||
| ) | ||
|
|
||
| const getTabBarHeight = React.useCallback( | ||
| (event: LayoutChangeEvent) => { | ||
| const height = event.nativeEvent.layout.height | ||
| if (tabBarHeight.value !== height) tabBarHeight.value = height | ||
| }, | ||
| [tabBarHeight] | ||
| ) | ||
|
|
||
| const onLayout = React.useCallback( | ||
| (event: LayoutChangeEvent) => { | ||
| const height = event.nativeEvent.layout.height | ||
| if (containerHeight.value !== height) containerHeight.value = height | ||
| }, | ||
| [containerHeight] | ||
| ) | ||
|
|
||
| const onTabPress = React.useCallback( | ||
| (name: TabName) => { | ||
| const i = tabNames.value.findIndex((n) => n === name) | ||
|
|
@@ -302,7 +299,7 @@ export const Container = React.memo( | |
| runOnUI(scrollToImpl)( | ||
| ref, | ||
| 0, | ||
| headerScrollDistance.value - contentInset.value, | ||
| headerScrollDistance.value - contentInset, | ||
| true | ||
| ) | ||
| } else { | ||
|
|
@@ -313,11 +310,14 @@ export const Container = React.memo( | |
| [containerRef, refMap, contentInset] | ||
| ) | ||
|
|
||
| React.useEffect(() => { | ||
| if (index.value >= tabNamesArray.length) { | ||
| onTabPress(tabNamesArray[tabNamesArray.length - 1]) | ||
| useAnimatedReaction( | ||
| () => tabNamesArray.length, | ||
| (tabLength) => { | ||
| if (index.value >= tabLength) { | ||
| runOnJS(onTabPress)(tabNamesArray[tabLength - 1]) | ||
| } | ||
| } | ||
| }, [index.value, onTabPress, tabNamesArray]) | ||
| ) | ||
|
Comment on lines
-316
to
+320
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is essentially the same as was just instead of running inside a useEffect it runs inside a useAnimatedReaction to avoid blocking the JS thread. |
||
|
|
||
| const pageScrollHandler = usePageScrollHandler({ | ||
| onPageScroll: (e) => { | ||
|
|
@@ -381,7 +381,7 @@ export const Container = React.memo( | |
| > | ||
| <Animated.View | ||
| style={[styles.container, { width }, containerStyle]} | ||
| onLayout={onLayout} | ||
| onLayout={getContainerLayoutHeight} | ||
| pointerEvents="box-none" | ||
| > | ||
| <Animated.View | ||
|
|
@@ -430,7 +430,7 @@ export const Container = React.memo( | |
| <AnimatedPagerView | ||
| ref={containerRef} | ||
| onPageScroll={pageScrollHandler} | ||
| initialPage={index.value} | ||
| initialPage={initialIndex} | ||
| {...pagerProps} | ||
| style={[pagerProps?.style, StyleSheet.absoluteFill]} | ||
| > | ||
|
|
||
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.
This change is safe to do because the initial value passed to useSharedValue will only be captured once on first pass, this is not reactive.