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

Mark adapter.dispose as async, Add dispose option to ignore disposing of the callAgent #5329

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

JamesBurnside
Copy link
Member

What

  • Mark adapter.dispose as async
  • Add dispose option to ignore disposing of the callAgent
  • Update useAdapter to await the dispose functions

Why

If the adapter is created through createadapterWithDeps we should not be disposing of the callAgent. However to remove this would be a breaking change so adding an option to dispose instead.

Marking the dispose as async is a breaking change, but technically this is already performing async behavior so we just need ARB sign off on this. Keeping in draft until ARB sign off.

How Tested

CI only

Copy link
Contributor

Copy link
Contributor

Calling bundle size is not changed.

  • Current size: 8139366
  • Base size: 8139366
  • Diff size: 0

Copy link
Contributor

CallWithChat bundle size is not changed.

  • Current size: 8139379
  • Base size: 8139379
  • Diff size: 0

Copy link
Contributor

github-actions bot commented Oct 17, 2024

Chat bundle size is decreased✅.

  • Current size: 1757519
  • Base size: 1757520
  • Diff size: -1

Copy link
Contributor

Copy link
Contributor

github-actions bot commented Oct 17, 2024

@azure/communication-react jest test coverage for stable.

Lines Statements Functions Branches
Base 26822 / 43181
62.11%
26822 / 43181
62.11%
733 / 1340
54.7%
2126 / 3415
62.25%
Current 26876 / 43181
62.24%
26876 / 43181
62.24%
733 / 1340
54.7%
2126 / 3423
62.1%
Diff 54 / 0
0.13%
54 / 0
0.13%
0 / 0
0%
0 / 8
-0.15%

Copy link
Contributor

github-actions bot commented Oct 17, 2024

@azure/communication-react jest test coverage for beta.

Lines Statements Functions Branches
Base 53767 / 87980
61.11%
53767 / 87980
61.11%
1093 / 2486
43.96%
3220 / 5324
60.48%
Current 53811 / 87990
61.15%
53811 / 87990
61.15%
1093 / 2486
43.96%
3194 / 5320
60.03%
Diff 44 / 10
0.04%
44 / 10
0.04%
0 / 0
0%
-26 / -4
-0.45%

Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant