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

Support react native #167

Merged
merged 7 commits into from
Mar 18, 2022
Merged

Support react native #167

merged 7 commits into from
Mar 18, 2022

Conversation

davidliu
Copy link
Contributor

No description provided.

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! could you install eslint in VS Code and take a look at the lint failures?

import Queue from './RequestQueue';

if (isWeb()) {
import('webrtc-adapter');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dynamic import is the main thing I'm unsure about. The webrtc-adapter library has a bunch of web specific code that crashes on react native, so needs to be omitted for RN. Didn't run into any issues on Chrome, but if there's a more proper way to do this, I'm all ears.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if there's a better way.. will test this for safari and FF to ensure things are still working.

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirming it works on FF/Safari too.

@davidliu davidliu merged commit 80407b9 into main Mar 18, 2022
@davidliu davidliu deleted the dl/react-native-rebase branch March 18, 2022 09:29
@davidliu davidliu restored the dl/react-native-rebase branch March 18, 2022 09:30
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