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

fix: thread manager bugs #1372

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

isekovanic
Copy link
Contributor

@isekovanic isekovanic commented Sep 27, 2024

CLA

  • I have signed the Stream CLA (required).
  • Code changes are tested

Description of the changes, What, Why and How?

This PR fixes 2 leftover issues in Threads v2:

  • ThreadManager state being stale after logging out in an app
  • Channel deletion not having a realtime effect on ThreadManager state

Changelog

@isekovanic isekovanic changed the title Fix/reset thread manager on disconnect fix: thread manager bugs Sep 27, 2024
Copy link
Contributor

github-actions bot commented Sep 27, 2024

Size Change: +925 B (+0.22%)

Total Size: 428 kB

Filename Size Change
dist/browser.es.js 92.8 kB +207 B (+0.22%)
dist/browser.full-bundle.min.js 54.3 kB +117 B (+0.22%)
dist/browser.js 94 kB +195 B (+0.21%)
dist/index.es.js 92.9 kB +208 B (+0.22%)
dist/index.js 94 kB +198 B (+0.21%)

compressed-size-action

src/client.ts Outdated Show resolved Hide resolved
@@ -117,6 +118,19 @@ export class ThreadManager<SCG extends ExtendableGenerics = DefaultGenerics> {
return () => unsubscribeFunctions.forEach((unsubscribe) => unsubscribe());
};

private subscribeChannelDeleted = () => {
const unsubscribeFunctions = ['notification.channel_deleted', 'channel.deleted'].map((e) => {
Copy link
Contributor

@arnautov-anton arnautov-anton Sep 30, 2024

Choose a reason for hiding this comment

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

Suggested change
const unsubscribeFunctions = ['notification.channel_deleted', 'channel.deleted'].map((e) => {
const unsubscribeFunctions = ['notification.channel_deleted'].map((e) => {

One should suffice, notification-type events are always delivered - with less information - but in this case it's fine. With two in the array we're going to run the handler twice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @arnautov-anton, notification events are not delivered for watched channels AFAIK. So we need to keep both.

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

Successfully merging this pull request may close these issues.

4 participants