react-native-bottom-sheet
react-native-bottom-sheet copied to clipboard
fix: replace getRefNativeTag with findNodeHandle
Please provide enough information so that others can review your pull request:
Motivation
Trying to fix this issue when new arch is enabled: https://github.com/gorhom/react-native-bottom-sheet/issues/1726
Current getRefNativeTag causes this issue with new architecture:
By reverting to findNodeHandle it works:
https://github.com/gorhom/react-native-bottom-sheet/assets/20777666/ea29dda2-987a-47a1-a4c3-6a569a8916f6
We had introduced getRefNativeTag because experimental new architecture last year did not support using findNodeHandle. This is no longer true with the proxy renderer they created.
This was tested with and without new architecture enabled on iOS.
tested v5 with/without new arch, it is not throwing this error
Really!? so it's unnecessary in v5, feel free to close the PR if so. @gorhom
@gorhom can confirm v5 is not entirely working on new arch. I'm getting this after switching to new arch (also tried updating from alpha 9 to 10)
it doesn't work with newArch=true even on 5.0.0-alpha.10 and RN 0.74.1 @gorhom
@ThusSpokeNomad @contactsimonwilson - please share a minimal reproducible example
@brentvatne
index.tsx
import { useState, useEffect } from "react";
import { Text } from "react-native";
import { GestureHandlerRootView } from "react-native-gesture-handler";
import WebView from "react-native-webview";
import BottomSheet, { BottomSheetScrollView } from "@gorhom/bottom-sheet";
export default () => {
const [lipsum, setLipsum] = useState<string>();
useEffect(() => {
void (async () => {
const lipsumJSON = await (await fetch("https://www.lipsum.com/feed/json")).json();
setLipsum(lipsumJSON.feed.lipsum);
})();
}, []);
return (
<GestureHandlerRootView>
<WebView source={{ uri: "https://www.lipsum.com/feed" }} />
<BottomSheet snapPoints={["50%", "100%"]}>
<BottomSheetScrollView>
<Text>{lipsum}</Text>
</BottomSheetScrollView>
</BottomSheet>
</GestureHandlerRootView>
);
};
package.json
{
"name": "minrepro",
"main": "expo-router/entry",
"scripts": {
"android": "expo run:android",
"ios": "expo run:ios"
},
"dependencies": {
"@gorhom/bottom-sheet": "^5.0.0-alpha.10",
"expo": "^51.0.9",
"expo-build-properties": "0.12.1",
"expo-router": "3.5.15",
"react": "18.2.0",
"react-native": "0.74.1",
"react-native-gesture-handler": "2.16.1",
"react-native-reanimated": "3.10.1",
"react-native-safe-area-context": "4.10.1",
"react-native-webview": "13.8.6"
},
"devDependencies": {
"@types/react": "18.2.79",
"typescript": "5.3.3"
}
}
also, interestingly, if you delete useState hook then it works fine (and useEffect could just console.log for testing).
I've also hit this issue on latest v5 and RN 0.74, it seems to happen only when using BottomSheetScrollView. This patch fixes it for me.
cc @gorhom re: @janicduplessis' comment - should we proceed with this pr?
@janicduplessis I don't see a getRefNativeTag.web in the source code.
Looks like it is only in the v5 branch, nvm then!
confirmed it solved the issue in my project with RN0.74.2 + New Arch
cc @gorhom - wdyt?
@AndreiCalazans sorry i have been very busy with work lately,, just to confirm is this fix backward compatible with old arch ?
@gorhom Yes. This proxy we included was a workaround for the new arch when it didn't have support for findNodeHandle - no longer required.
thanks for confirming @AndreiCalazans , this fix should be out for the next releases of v4 and v5