Skip to content

Commit

Permalink
fix(client): One cleanup per subscription (enisdenjo#67)
Browse files Browse the repository at this point in the history
* Prevent canceller from being invoked twice for same subscription

* apply suggestions

* fix: remove canceller on close

Co-authored-by: enisdenjo <badurinadenis@gmail.com>
  • Loading branch information
rpastro and enisdenjo committed Nov 12, 2020
1 parent 3f4f836 commit 5a5ae4d
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/client.ts
Expand Up @@ -243,12 +243,14 @@ export function createClient(options: ClientOptions): Client {

state.socket.addEventListener('close', listener);
function listener(event: CloseEvent) {
cancellerRef.current = null;
state.locks--;
state.socket?.removeEventListener('close', listener);
return reject(event);
}

cancellerRef.current = () => {
cancellerRef.current = null;
cleanup?.();
state.locks--;
if (!state.locks) {
Expand Down Expand Up @@ -369,12 +371,14 @@ export function createClient(options: ClientOptions): Client {

socket.addEventListener('close', listener);
function listener(event: CloseEvent) {
cancellerRef.current = null;
state.locks--;
socket.removeEventListener('close', listener);
return reject(event);
}

cancellerRef.current = () => {
cancellerRef.current = null;
cleanup?.();
state.locks--;
if (!state.locks) {
Expand Down Expand Up @@ -539,8 +543,7 @@ export function createClient(options: ClientOptions): Client {
}
})()
.catch(sink.error)
.then(sink.complete) // resolves on cancel or normal closure
.finally(() => (cancellerRef.current = null)); // when this promise settles there is nothing to cancel
.then(sink.complete); // resolves on cancel or normal closure

return () => {
cancellerRef.current?.();
Expand Down

0 comments on commit 5a5ae4d

Please sign in to comment.