Skip to content

chat: add disposal spec - #352

Merged
AndyTWF merged 2 commits into
mainfrom
chat-disposal-methods
Jul 22, 2025
Merged

chat: add disposal spec#352
AndyTWF merged 2 commits into
mainfrom
chat-disposal-methods

Conversation

@AndyTWF

@AndyTWF AndyTWF commented Jul 20, 2025

Copy link
Copy Markdown
Contributor

Adds a specification for disposing of the Chat Client and its rooms instance.

CHA-1054

@AndyTWF
AndyTWF requested review from a team, JoaoDiasAbly and vladvelici and removed request for a team July 20, 2025 12:25
@github-actions
github-actions Bot temporarily deployed to staging/pull/352 July 20, 2025 12:26 Inactive
@AndyTWF
AndyTWF force-pushed the chat-disposal-methods branch from 7d50ef7 to d824031 Compare July 20, 2025 14:33
@github-actions
github-actions Bot temporarily deployed to staging/pull/352 July 20, 2025 14:33 Inactive
Comment on lines +301 to +310
h2(#roos-disposal). Rooms Disposal

Rooms in the Chat SDK are a complex type that register many different listeners and consume many different events. Therefore, there must be methods available to clean up any listeners to allow for garbage collection.

** @(CHA-RD1)@ @[Testable]@ The @Rooms@ map / manager must offer an internal @dispose@ method that must prepare the rooms manager for garbage collection and release all managed rooms.
*** @(CHA-RD1a)@ @[Testable]@ When @dispose@ is called, it must mark the instance as disposed to prevent future room operations.
*** @(CHA-RD1b)@ @[Testable]@ If no rooms exist when @dispose@ is called, the method must complete successfully without error.
*** @(CHA-RD1c)@ @[Testable]@ If rooms exist in the map, the @dispose@ method must release all rooms currently in the rooms map concurrently.
*** @(CHA-RD1d)@ @[Testable]@ The @dispose@ method must wait for all room release operations to complete before resolving.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought the point of the spec generally is public API. These points describe how things work internally. Do we want these in the spec?

@AndyTWF AndyTWF Jul 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Whilst we've tried to focus it on the public API where possible, there are occasions with complex internal behaviour that needs specifying to avoid implementation divergence (e.g. RTL3d specifies performing an internal attach operation where necessary). We did the same with the room lifecycle manager.

By having a spec point for the internal GC methods, we're ensuring appropriate behaviour in response to the public API

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What I have done though is 32dcbe3 which makes it a bit less prescribe-y on the API, to make it clear that it's an internal impl mechanism.

@vladvelici vladvelici left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's quite a bit of internal API in this. Shouldn't the spec mostly focus on the public API and behaviour we expect from the SDK when interacting with this public API?

@AndyTWF
AndyTWF merged commit 84faabe into main Jul 22, 2025
2 checks passed
@AndyTWF
AndyTWF deleted the chat-disposal-methods branch July 22, 2025 12:26
ttypic pushed a commit that referenced this pull request Mar 6, 2026
ttypic pushed a commit that referenced this pull request Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants