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

All Drei imports are broken on react-native because of navigator.userAgent check in video texture #1917

Closed
jorjordandan opened this issue Apr 12, 2024 · 7 comments · Fixed by #1919
Labels
bug Something isn't working released

Comments

@jorjordandan
Copy link

jorjordandan commented Apr 12, 2024

"dependencies": {
"@react-three/drei": "^9.105.3",
"@react-three/fiber": "^8.16.1",
"expo": "~50.0.14",
"expo-gl": "~13.6.0",
"expo-status-bar": "~1.11.1",
"react": "18.2.0",
"react-native": "0.73.6",
"three": "^0.163.0"
}

Problem description:

Trying to use any import from @react-three/drei/native in react-native causes error "TypeError: Cannot read property 'toLowerCase' of undefined, js engine: hermes" referencing the hls.js library, which was recently added to videoTexture. This is because of the check var chromeOrFirefox = /chrome|firefox/.test(navigator.userAgent.toLowerCase()); in hls.js - React native has no navigator defined.

References addition of hls support here: 47d94d7

Relevant code:

Example

import { StatusBar } from 'expo-status-bar';
import { StyleSheet, Text, View } from 'react-native';

import { Suspense } from 'react'
import { Canvas } from '@react-three/fiber/native'
import { Box } from '@react-three/drei/native'


export default function App() {
  return (
    <View style={styles.container}>
      <Text>You will get a 'tolowercase' error. </Text>
      <StatusBar style="auto" />
      <Canvas width={500} height={500}>
      <ambientLight />
      <Suspense>
        <Box />
      </Suspense>
    </Canvas>
      
    </View>
  );
}

Suggested solution:

Since video texture relies on dom video elements, I don't think it would work at all in react-native. The easiest fix might be to check for navigator and do an early return if it's undefined, maybe console.warn explaining that react-native doesn't currently support video-texture.

Eventually, it might make more sense if it's located in /web rather than /core, but that is probably a breaking change, or maybe there's some other consideration I don't know about.

Happy to create a PR if needed :)

@jorjordandan jorjordandan added the bug Something isn't working label Apr 12, 2024
jorjordandan added a commit to jorjordandan/drei that referenced this issue Apr 12, 2024
fixes pmndrs#1917. Returns a dummy hook when navigator or window undefined.
@jorjordandan
Copy link
Author

jorjordandan commented Apr 12, 2024

hmm no that fix won't work... as long as hls.js is being imported it will create the error 😅

@jorjordandan
Copy link
Author

@netgfx - do you have any suggestions? Is it possible to move this to /web? looks like index points to web anyways so it wouldn't be a breaking change

jorjordandan added a commit to jorjordandan/drei that referenced this issue Apr 12, 2024
fixes pmndrs#1917.
Moves useVideoTexture to /web. Also fix reference from faceControls.
@netgfx
Copy link
Contributor

netgfx commented Apr 12, 2024

@jorjordandan Hey there, do you mean move the useVideoTexture to web? I guess it makes sense if it is completely useless for RN, but not my call. @CodyJasonBennett what do you think?

I tried to search for a solution but HLS is "special"

  • It needs to be initialized and bound to video element before the video is assigned to the texture, so initializing it inside a useEffect won't work
  • It can be dynamically imported and this will work but due to the async nature of the dynamic import hlsRef is never read correctly so it can be destroyed on component unmount (it is always undefined).
  • The only solution I found in order to teardown the hls object correctly on unmount was to directly assign it on the ref, but that means that the import needs to happen sync not async.

Currently I don't see how this can be modified to work with RN as is. If we break it down to two individual components one with hls and one without we could potentially make it work via lazy loading the components themselves based on some conditional with the navigator bu that would mean creating at least one more file and the drei structure uses self-contained components.

@jorjordandan
Copy link
Author

Thanks @netgfx! As far as I can tell it's not possible for it to be used with react-native. I've created a pull request to move it which will fix the issue.

@CodyJasonBennett
Copy link
Member

CodyJasonBennett commented Apr 13, 2024

Can you give #1919 a try? I'm keeping useVideoTexture in core, but gating use of hls.js behind feature checks.

npm i https://pkg.csb.dev/pmndrs/drei/commit/4e9de682/@react-three/drei

@jorjordandan
Copy link
Author

Yup, that fixes it, thanks!

Copy link

🎉 This issue has been resolved in version 9.105.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
3 participants