Filtered getItem seems to be emitting something

[sylvaingirardbe]

Something really strange is going on with getItem from LocalStorage.

A minimal reproduction of the issue can be found at https://github.com/sylvaingirardbe/local-storage-repro.

If you consider this code from app.component.ts

    const localStorage$ = this.localStorage.getItem('token').pipe(
      filter(token => token != undefined && token != null && token != '' && token != 'NO_TOKEN'),
    );

    const queryString$ = this.route.queryParams.pipe(
      filter(params => params.token),
      map(params => params.token),
    );

    const interval$ = interval(3000).pipe(
      first(),
      map(_ => 'NO_TOKEN'),
    );

    this.subscriptions.add(
      race(
        interval$,
        localStorage$,
        queryString$
      )
        .subscribe(token => {
          console.log('Token', token);
        })
    );

It should log out NO_TOKEN after 3 seconds if token is not set. If you run the code, you'll see it doesn't. Commenting out localStorage$ in race fixes the issue but obviously that's not an option.

If you play around with the interval (lowering it to 200, for instance) you'll see that NO_TOKEN is logged out. Increasing it to something like 500 prevents NO_TOKEN from being logged out.

All this makes me think that race is seeing some sort of emission from localStorage$.

Behaviour is the same in both versions 5 and 6.

Any ideas?

[cyrilletuzi]

What do you want to achieve with this code? Trying to get the token from queryParams first, then try from localStorage and fail after some timeout?

If so, it's a misunderstanding of RxJS:

1. The race operator will take the first Observable which will emit and discard the others, so yes it's normal that depending on the timeout, it will emit different things, as all your Observables here are asynchronous and you don't know how much time the localStorage and queryParams ones will take.

2. A timeout is not necessary in this case. There is no reason querying localStorage or queryParams to not complete.

3. queryParams is an Observable that can emit multiple times (if query params change), while the others emit just once (it's why you put first on the timeout one). Then, either you use first on the queryParams one too, or you don't need the queryParams Observable but just the this.route.snapshot.queryParams.get('token').

[sylvaingirardbe]

What I'm trying to do is provide a default token when there isn't one in local storage and there isn't one on the querystring. In both cases, those observables shouldn't emit anything and the interval should be the one to win the race.

What isn't provided in the reproduction is the rest of the flow. When race emits, the token is set in application state and in local storage so the next time the application is loaded, the token can be retrieved from local storage to be set on the application state again. Come to think of it that this solution might not be as perfect as thought of at first.

But regardless, when there is nothing in local storage and nothing on the querystring, I'm expecting the interval to win the race at any time set and that's not happening when it is set at +500ms.

[cyrilletuzi]

When there is a token already: race will take the first Observable to emit, no matter the order. But the order is important in what you want: 1/ queryParams or 2/ localStorage or 3/ default value. So you want to chain, not to race.

When there is not a token yet: I suspect that even if you filter when there is no token, the localStorage Observable completes (and that is normal), and then win the race. But all of this is due to the previous point: you race instead of chaining.

You want something like this:

of(this.route.snapshot.queryParams.get('token')).pipe(
  switchMap((token) => token ? of(token) : this.localStorage.getItem('token')),
  map((token) => token ? token : 'NO_TOKEN')
);

Closing as it's not a lib issue, but feel free to comment if you find a problem.

[sylvaingirardbe]

Ok, I wasn't aware that the observable completes instead of not emitting anything. Thanks for clearing that out.