-
Notifications
You must be signed in to change notification settings - Fork 73
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
[Bug fix] CallId switch losing participants fix #5725
Conversation
packages/react-composites/src/composites/CallComposite/hooks/useAdaptedSelector.ts
Outdated
Show resolved
Hide resolved
@@ -68,7 +62,7 @@ export const useSelectorWithAdaptation = < | |||
if (!mounted.current) { | |||
return; | |||
} | |||
const newProps = selector(adaptState(state), selectorProps ?? callConfigProps); | |||
const newProps = selector(adaptState(state), selectorProps ?? { callId: state.call?.id }); |
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.
I assume it's ok { callId: state.call?.id }
will be a new object each time selector
is called, but perhaps we should check that selector
from reselect doesn't require the props to be memoized
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.
Looking at their docs for create selector it does not seem that they need anything to be memoized. they have lots of options to memoize the selector though
https://reselect.js.org/api/createSelector
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.
Just verified this, put a console log inside the if (propRef.current !== newProps) {
(and one in an else
to that) and validate there is no difference if a cached { callId }
is used vs creating the callId object each pass 👍
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-aprplovfpu.chromatic.com/ |
@azure/communication-react jest test coverage for stable.
|
@azure/communication-react jest test coverage for beta.
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-afvvgfsggw.chromatic.com/ |
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-odnabugnar.chromatic.com/ |
…communication-ui-library into dmceachernmsft/callid-fix
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-fbvszaxyto.chromatic.com/ |
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-uxuybzpdrv.chromatic.com/ |
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-wyxzlzznto.chromatic.com/ |
CallWithChat bundle size is not changed.
|
Calling bundle size is not changed.
|
Chat bundle size is not changed.
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-drrdfhjvgy.chromatic.com/ |
What
Fix issue where useAdaptedSelector is using a stale callId when the callId updates in the call when joining a group call
Why
Resolves an issue where the participant loses the other people in the call until the next state change in the adapter
https://skype.visualstudio.com/SPOOL/_workitems/edit/4005860
How Tested
Validated locally