-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(ENGDESK-27107): Upgrade WebRTC library to support React 18 #321
Conversation
7f717ea
to
52f8ca7
Compare
packages/react-client/package.json
Outdated
@@ -20,6 +20,7 @@ | |||
"build": "microbundle-crl --no-compress --format modern,cjs", | |||
"start": "microbundle-crl watch --no-compress --format modern,cjs", | |||
"prepare": "install-peers && run-s build", | |||
"install-peers": "install-peers", |
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.
Do we need this? It seems like this would install the peerDependencies as devDependencies, according to the documentation: https://www.npmjs.com/package/install-peers
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.
it was already there as you can see on the line above.
|
||
if (props?.onNotification) { | ||
telnyxClient!.on('telnyx.notification', props.onNotification); | ||
} | ||
} | ||
}, []); |
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.
Do you need to add telnyxClient
as a dependency or will this have client on initial render?
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.
thanks for cathing it, I added.
📝 To Do
✋ Manual testing
packages/react-client
🦊 Browser testing
Desktop