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: Fix types for ReactVideoSource to also allow require(..) sources #3354

Closed
wants to merge 5 commits into from

Conversation

mrousavy
Copy link
Contributor

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.

…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
Copy link
Collaborator

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 🙏

@freeboub
Copy link
Collaborator

This Pr doesn't pass the linter step.
you can check locally with "yarn lint" command.
Can you please check ?

notice that ideally we should move require option inside uri prop, but that is another subject !

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
mrousavy and others added 2 commits November 17, 2023 14:52
Co-authored-by: Olivier Bouillet <[email protected]>
Co-authored-by: Olivier Bouillet <[email protected]>
@mrousavy
Copy link
Contributor Author

Oh my bad I only checked typescript output - will fix linting now!

@freeboub
Copy link
Collaborator

@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 ?
Thank you

@freeboub
Copy link
Collaborator

My Pr has been merged, don't hesitate to comment on it if you see any issues with it !
Thank you !

@freeboub freeboub closed this Nov 18, 2023
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.

2 participants