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.

Sign up for GitHub

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

Jump to bottom

Find a way to clean up arrays of subscriptions #36

Closed
lawrence-forooghian opened this issue Sep 3, 2024 · 0 comments · Fixed by #207
Closed

Find a way to clean up arrays of subscriptions #36

lawrence-forooghian opened this issue Sep 3, 2024 · 0 comments · Fixed by #207
Assignees
@lawrence-forooghian
Labels
enhancement New feature or improved functionality.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Sep 3, 2024 •
edited
Loading

In #19 the DefaultRoomStatus creates an array of Subscription objects to which it can emit status updates. We need a way to remove subscriptions from this array once they're no longer needed. In 20e7f5f I said that there might be a way to do this automatically based on some event (I’m being very vague here because I don't know what); let's look into that. Else we might need to introduce the unsubscribe methods from JS.

We also need to clean up the underlying ably-cocoa listeners where relevant.

┆Issue is synchronized with this Jira Story by Unito
@lawrence-forooghian lawrence-forooghian added enhancement New feature or improved functionality. bug Something isn’t working. It’s clear that this does need to be fixed. and removed enhancement New feature or improved functionality. labels Sep 3, 2024
@lawrence-forooghian lawrence-forooghian mentioned this issue Oct 9, 2024
Merged
@lawrence-forooghian lawrence-forooghian added enhancement New feature or improved functionality. and removed bug Something isn’t working. It’s clear that this does need to be fixed. labels Oct 10, 2024
@coderabbitai coderabbitai bot mentioned this issue Oct 15, 2024
Merged
@coderabbitai coderabbitai bot mentioned this issue Oct 28, 2024
Merged
This was referenced Nov 11, 2024
Merged
Merged
Merged
Merged
This was referenced Nov 18, 2024
Merged
Merged
Merged
Merged
@coderabbitai coderabbitai bot mentioned this issue Nov 27, 2024
Merged
lawrence-forooghian added a commit that referenced this issue Dec 2, 2024
@lawrence-forooghian
Rename Subscription’s finish method to unsubscribe
300f5f8 
I don’t think that this method should exist at all; the intention for
our public API, as described in 20e7f5f, is that the user should not
need to explicitly unsubscribe. (We have #36 for figuring out how to
implement the appropriate cleanup.) However, this method does now exist,
and we’re probably not going to get rid of it before beta, so we agreed
in standup today to at least name it in line with JS.
@lawrence-forooghian lawrence-forooghian mentioned this issue Dec 2, 2024
Merged
@coderabbitai coderabbitai bot mentioned this issue Dec 4, 2024
Merged
lawrence-forooghian added a commit that referenced this issue Dec 4, 2024
@lawrence-forooghian
Rename Subscription’s finish method to unsubscribe
501a1d5 
I don’t think that this method should exist at all; the intention for
our public API, as described in 20e7f5f, is that the user should not
need to explicitly unsubscribe. (We have #36 for figuring out how to
implement the appropriate cleanup.) However, this method does now exist,
and we’re probably not going to get rid of it before beta, so we agreed
in standup today to at least name it in line with JS.
@coderabbitai coderabbitai bot mentioned this issue Dec 5, 2024
Merged
@lawrence-forooghian lawrence-forooghian self-assigned this Dec 18, 2024
lawrence-forooghian added a commit that referenced this issue Jan 8, 2025
@lawrence-forooghian
Clean up subscriptions
2554e1a 
Hook in to AsyncStream’s termination notification mechanism, so that
when the user discards a subscription or cancels the task in which
they’re iterating over a subscription, we:

- remove this subscription from our internal data structures
- remove any corresponding ably-cocoa listeners that drive this
  subscription

I’m sure that there will turn out to be a bunch of wrong stuff that I’ve
done here, due to my still-shaky knowledge of concurrency stuff and
AsyncSequence best practices, but it’s a start.

Resolves #36.
@lawrence-forooghian lawrence-forooghian mentioned this issue Jan 8, 2025
Merged
lawrence-forooghian added a commit that referenced this issue Jan 8, 2025
@lawrence-forooghian
Clean up subscriptions
9bf7331 
Hook in to AsyncStream’s termination notification mechanism, so that
when the user discards a subscription or cancels the task in which
they’re iterating over a subscription, we:

- remove this subscription from our internal data structures
- remove any corresponding ably-cocoa listeners that drive this
  subscription

I’m sure that there will turn out to be a bunch of wrong stuff that I’ve
done here, due to my still-shaky knowledge of concurrency stuff and
AsyncSequence best practices, but it’s a start.

Resolves #36.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lawrence-forooghian lawrence-forooghian

Labels
enhancement New feature or improved functionality.
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ECO-4963] Clean up when user finishes with subscription ably/ably-chat-swift
1 participant
@lawrence-forooghian