Allow custom reconnectionPolicy for createReconnectEffect

[tscislo]

I'd like to create PR for this repo with following change to createReconnectEffect. Please have a look @Odonno and let me know how can I create PR?

Basically - purpose of it is to limit retryTimes, also isOnline validation is done before each retry, as there is no point in reconnecting when browser is offline.

export const createReconnectEffect = (actions$: Actions<Action>, intervalTimespan: number, retryTimes: number = 10) => {
  return createEffect(() =>
    actions$.pipe(
      ofType(signalrDisconnected),
      groupBy(action => action.hubName),
      mergeMap(group =>
        group.pipe(
          exhaustMapHubToAction(({action}) => {
              return timer(0, intervalTimespan)
                .pipe(
                  switchMap(isOnline),
                  filter((is) => is),
                  take(retryTimes),
                  map(_ => reconnectSignalRHub(action)),
                  takeUntil(
                    this.actions$.pipe(
                      ofType(signalrConnected),
                      ofHub(action)
                    )
                  )
                );
            }
          )
        )
      )
    )
  );
};

[Odonno]

Well, I see what got you here.

I already refactored the createReconnectEffect function, using an options as 2nd parameter. But now what I would want is to allow devs to trigger the reconnection by themselves, using their own implementation. That was what I first intended but not knowing the best way to do it.

Now that I see more clearly, I would suggest to add another property in the options called triggerReconnection$ which would be an Observable.

If the Observable is not provided, I will use the default implementation (current one). What do you think?

[tscislo]

In order to currently achieve above mentioned result (limiting number of retries) I had to copy this whole implementation of effect into my source code. So with what you're proposing (triggerReconnection$) every dev would also need to copy lot of boilerplate code (things that always will be needed: ofType, groupBy, etc).

Therefore my proposal is to have options passed like this (let's have both):

{
intervalTimespan
retryTimes,
triggerReconnection$
}

Which would allow to have completely custom triggerReconnection strategy for those who want and to have fully working, yet customizable strategy for those who want it out of the box, agreed?

[Odonno]

Ah sorry.

The triggerReconnection will only be inside the exhaustMapHubToAction. So it will look like this:

export const createReconnectEffect = (actions$: Actions<Action>, options) => {
  return createEffect(() =>
    actions$.pipe(
      ofType(signalrDisconnected),
      groupBy(action => action.hubName),
      mergeMap(group =>
        group.pipe(
          exhaustMapHubToAction(({action}) => {
              return options.triggerReconnection(action)
                .pipe(
                  map(_ => reconnectSignalRHub(action)),
                  takeUntil(
                    this.actions$.pipe(
                      ofType(signalrConnected),
                      ofHub(action)
                    )
                  )
                );
            }
          )
        )
      )
    )
  );
};

[Odonno]

I just edited my code to a function instead. Of course you could create a custom reconnection strategy based on the hub (based on the disconnected action previously dispatched).

[tscislo]

Ok in that case, that's all we need I guess. Will you progress with those changes?

[Odonno]

I will do it right now :)

[Odonno]

Done.

You should see the v8.4.0 now. Tell me what you think!

[tscislo]

Yes, that works as expected. Thank you!