PR for offSignalRHub action

[tscislo]

Using this lib we found that we're missing offSignalRHub action. I have PR ready with that change to be merge. However I cannot push to this repo. Please advise @Odonno

[Odonno]

Hello @tscislo

What do you mean by offSignalRHub action? What is your intent?

[mcr85]

@Odonno we would like to have similar API for unsubscribing from SignalR events as we have for disconnecting from SignalR hub. We have stopSignalRHub action for disconnecting, but no action for unsubscribing from hub's SignalR events.

It would be convenient to clear subscription and then kill connection by dispatching two actions. First for unsubscribe, second for disconnect.

As I see it, it's necessary to clear subscriptions before disconnecting from hub. Otherwise one could easily end up with duplicated subscriptions to same SignalR event.

[tscislo]

@Odonno @mcr85 In its simplest form, such event could look like this. Essential part is hub.off(...), currently there is no easy way to dispach an event to trigger hub.off(...)

  offHub$ = createEffect(() =>
    this.actions$.pipe(
      ofType(offSignalRHub),
      map(({hubName, url, eventName}) => {
        return {
          hub: findHub({hubName, url}),
          eventName
        };
      }),
      tap(({hub, eventName}) => hub.off(eventName)),
      map(({hub, eventName}) => {
        return signalrHubOff(hub);
      })
    )
  );

[Odonno]

Oh, I see.

The problem is that the on event function is not independant and it is required to be used with its counterpart, the off function.

In order to make things simpler, I would go this way :

• remove the off function definitely
• when calling on function let it have its self subscription so the subscription is automatically completed once
  1. the outer subscription is completed
  2. the connection is closed

This way you can freely subscribe to an event at anytime (if the hub is active) and the listener/subscription is cleaned the easy way (when hub is disconnected for example).

That will also change the way we subscribe to events. The on should be called after, and only after the hub is started.

effect$ = createEffect(() =>
  this.actions$.pipe(
    ofType(signalrHubUnstarted), // SHOULD BECOME 'signalrConnected'
    mergeMapHubToAction(({ hub }) => {
      // TODO : add event listeners
      const whenEvent$ = hub.on("eventName").pipe(map((x) => createAction(x)));

      return merge(whenEvent$, ...); // hub already started from another effect
    })
  )
);

What do you think?

[tscislo]

@Odonno thanks for your input!

This exactly is the problem we're trying to solve: "The problem is that the on event function is not independant and it is required to be used with its counterpart, the off function." Currently there is no easy way to trigger hub.off() function, therefore in our microfrontend solution we ended up being subscribed on hub even after the app is unmounted.

How we achieve that goal is debatable ;)

Also quite easy and without braking any of current apps and contracts would be just to call hub.off() on stopSignalRHub action, because this is essentaially what we're missing I guess. My expectation would be that when hub is stopped it also unsubscribes using hub.off().

What do you think?

I don't fully understand your solution. I would need to see a PR to really say something about that.

[Odonno]

Ok, we are on the right track.

The thing is that adding a new action will add some complexity to the library which is already a little too much IMO. But, yes, having independant subscription using the on function is a major breaking change.

I will work on a PR asap so you see what it would look like.

[Odonno]

I have started the PR here : https://github.com/Odonno/ngrx-signalr-core/pull/74

As you can see, I updated the readme file so you can have a glimpse to the change. Previously, you needed at least 2 effects to make it work now you need at least 3 :

• one to create the hub
• one to start the hub
• and now one to listen to the events (using the on function) when the hub is connected (or any other event that would trigger the subscription)

The on subscription is also self terminated when either :

1. the connection is closed (like manually or when an error occured)
2. the outer Observable/subscription completed

I will update the samples based on this change and then validate this PR.

What do you think about the code change? Feel free to ask any question.

[Odonno]

Samples updated. I'll let you see the changes and try the samples if you like. Personally, I really like this change.

[donohoea]

My expectation would be that when hub is stopped it also unsubscribes using hub.off().

I agree, my expectation would be that when the hub is stopped the event listener effects are unsubscribed. It seems like this PR achieves that. Thanks! Looking forward to the update.

[tscislo]

@Odonno we tested your change and it fits our needs. Please merge that and publish new version.

[Odonno]

Published! You can now use the version 13.1.0