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

fix: move useVideoTexture to /web #1918

Conversation

jorjordandan
Copy link

fixes #1917.
Moves useVideoTexture to /web. Also fix reference from faceControls.

Why

Currently, useVideo texture imports hls.js. This import triggers a check to navigator, which causes an error because navigator doesn't exist on react native. It's not possible to dynamically import it without making it into an async function which would cause other issues.

What

I've moved useVideoTexture from core to web, since it does not work on native anyways.

Checklist

  • [x ] Ready to be merged

Also fixed one reference to useVideoTexture from faceControls.

fixes pmndrs#1917.
Moves useVideoTexture to /web. Also fix reference from faceControls.
Copy link

vercel bot commented Apr 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
drei ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 12, 2024 9:21pm

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@CodyJasonBennett
Copy link
Member

Continued in #1919. Thanks for the help!

@netgfx
Copy link
Contributor

netgfx commented Apr 13, 2024

@CodyJasonBennett Thanks for this, I tried something similar but keep in mind that the hlsRef on the useEffect is null so I think this cleanup:

return () => {
    if (hlsRef.current) {
      hlsRef.current.destroy()
      hlsRef.current = null
    }
  }

will never run.

@CodyJasonBennett
Copy link
Member

If hlsRef is ever non-null before unmount, that will run. #1919 doesn't alter cleanup behavior.

@jorjordandan jorjordandan deleted the fix/1917-useVideoTexture-react-native branch April 15, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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