-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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: Fix types for ReactVideoSource
to also allow require(..)
sources
#3354
Conversation
…rces We need to be careful here to not use `any`, so I used `NodeRequire` - which is afaik present in all React Native environments as a type.
CHANGELOG.md
Outdated
@@ -27,6 +27,8 @@ | |||
|
|||
## Next | |||
- Android, iOS: add onVolumeChange event #3322 | |||
- Fix types for ReactVideoSource to also allow require(..) sources |
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.
you can remove this change for now, I will fix PR template, but thank for it 🙏
This Pr doesn't pass the linter step. notice that ideally we should move require option inside uri prop, but that is another subject ! |
Co-authored-by: Olivier Bouillet <[email protected]>
Co-authored-by: Olivier Bouillet <[email protected]>
Oh my bad I only checked typescript output - will fix linting now! |
@mrousavy I rework the fix here: https://github.com/react-native-video/react-native-video/pull/3361/files can you please check if it works for you ? |
My Pr has been merged, don't hesitate to comment on it if you see any issues with it ! |
We need to be careful here to not use
any
, so I usedNodeRequire
- which is afaik present in all React Native environments as a type.