-
Notifications
You must be signed in to change notification settings - Fork 551
docs(driver): Update disconnect event comment #24897
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
docs(driver): Update disconnect event comment #24897
Conversation
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.
Pull Request Overview
This PR updates the documentation and changelog to reflect that the disconnect
event’s reason
parameter now accepts undefined
, supporting clean, non-error disconnections.
- Refined JSDoc on the
disconnect
event inIDocumentDeltaConnectionEvents
and updated its TypeScript signature to(reason: IAnyDriverError | undefined)
. - Expanded the affected packages list in the changeset and clarified changelog wording.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/common/driver-definitions/src/storage.ts | Updated JSDoc and method signature for disconnect to allow undefined |
.changeset/fuzzy-turkeys-help.md | Added new packages and adjusted changelog text around reason handling |
Comments suppressed due to low confidence (2)
packages/common/driver-definitions/src/storage.ts:245
- [nitpick] Since the interface signature has already been updated in this release, consider updating the wording from 'will change' to reflect that the signature has changed, or remove this note to avoid confusion.
* Signature will change from `(reason: IAnyDriverError) => void` to `(reason: IAnyDriverError | undefined) => void`.
.changeset/fuzzy-turkeys-help.md:16
- [nitpick] This sentence may be confusing since accepting
undefined
is already introduced in this release; consider rephrasing or removing it.
In a future release, the `reason` parameter will also accept `undefined`.
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.
A few comments, but looks good!
Seems like there was an announcement regarding this in the last release? Should it be removed from the release notes? https://github.com/microsoft/FluidFramework/releases/tag/client_v2.43.0#user-content-the-reason-parameter-on-the-disconnect-event-is-now-optional-to-allow-for-clean-non-error-disconnections-24840 |
Yes Thank you for letting me know, ill change this |
updated https://github.com/microsoft/FluidFramework/releases/tag/client_v2.43.0, removing |
This PR updates documentation to reflect that the
reason
parameter on thedisconnect
event inIDocumentDeltaConnectionEvents
will acceptundefined
to support clean, non-error disconnections.The
reason
parameter in disconnect event listeners now acceptsundefined
:(reason?: IAnyDriverError) => void
(reason: IAnyDriverError | undefined) => void
1. Updated Interface Documentation
disconnect
event inIDocumentDeltaConnectionEvents