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

jorjordandan commented 2 months ago

"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: https://github.com/pmndrs/drei/commit/47d94d72f70ad23818b993f8b316c132ff04e146

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 commented 2 months ago

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

jorjordandan commented 2 months ago

@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

netgfx commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Yup, that fixes it, thanks!

github-actions[bot] commented 2 months ago

:tada: This issue has been resolved in version 9.105.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

pmndrs / drei