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.

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

switchMap not unsubscribing depending on import of Observable #3306

Closed
Airblader opened this issue Feb 8, 2018 · 11 comments
Closed

switchMap not unsubscribing depending on import of Observable #3306

Airblader opened this issue Feb 8, 2018 · 11 comments

Comments

@Airblader
Copy link
Contributor

@Airblader Airblader commented Feb 8, 2018

RxJS version: 5.5.6

In our app, we today encountered an issue with code which is essentially the same as the following.

[Removed in favor of the repository with a reproducable version posted below]

The behavior we saw was that all the (temporarily) started timers kept executing in parallel, as if switchMap didn't unsubscribe from it when id$ emitted.

After a lot of debugging, I eventually figured out that the bug only arises when Observable is imported as

import { Observable } from "rxjs";

but not when imported as

import { Observable } from "rxjs/Observable";

Unfortunately, I cannot reproduce the issue on Stackblitz independently of how I import. My feeling here is that there is some dependency which patches Observable, perhaps with an older, faulty version or something.

However, the problem I am having with this theory: I would think that in this case it breaks when importing from rxjs/Observable, but not when importing rxjs. But what we see is exactly the opposite.

I realize that with rxjs 5.5.0, patching Observable isn't the choice of things anymore and we do intend to move away from this. In the mean time, I would really like to understand how importing from rxjs breaks behavior so drastically and would appreciate any ideas on how to figure out what is messing with us here.

@Airblader

This comment has been minimized.

@kayjtea

This comment has been minimized.

Copy link

@kayjtea kayjtea commented Feb 16, 2018

It's definitely the same underlying issue.

Importing as

import { Observable } from "rxjs/Observable";

also makes my issue go away.

@kayjtea

This comment has been minimized.

Copy link

@kayjtea kayjtea commented Feb 16, 2018

This is getting stranger. Importing as

import { Observable } from "rxjs/Rx";

also makes the problem go away. So one goes through package.json and the other doesn't but I don't see why that should matter. I'm suspecting it has something to do with tree shaking. Will upload reproduce repo this weekend.

@Airblader

This comment has been minimized.

Copy link
Contributor Author

@Airblader Airblader commented Feb 17, 2018

I'd look at the package-lock instead of only the direct dependencies. I do remember seeing "rx" in there which to my understanding is rxjs in an older version. But as explained in my first post I still can't make sense of it.

@kayjtea

This comment has been minimized.

Copy link

@kayjtea kayjtea commented Feb 20, 2018

Reproduce steps:

https://github.com/kayjtea/rxjs3306

The problem arises from mixing the full RxJS import with the patch mechanism.

@Airblader

This comment has been minimized.

Copy link
Contributor Author

@Airblader Airblader commented Feb 20, 2018

Interestingly enough, running the example on Stackblitz (https://stackblitz.com/github/kayjtea/rxjs3306) doesn't seem to reproduce the problem again.

@michaelbromley

This comment has been minimized.

Copy link

@michaelbromley michaelbromley commented Mar 8, 2018

I ran into the same issue just now.

My code is something like the following:

import {Observable} from 'rxjs';
// ....

Observable.combineLatest(paginationConfig$, filters$)
  .switchMap(([config, filters]) => this.api.getList(config, filters))
  .subscribe();

When I would change filters, the API call to getList is not cancelled (i.e. the observable is not being unsubscribed).

Changing to import to "rxjs/Observable" as described above also fixes the issue for me.

Additional info:

$ npm ls rxjs
source@0.0.0 C:\Development\kbart\kbui\source
+-- @angular/cli@1.6.3
| +-- @angular-devkit/schematics@0.0.42
| | `-- rxjs@5.5.6  deduped
| `-- rxjs@5.5.6  deduped
`-- rxjs@5.5.6
@benlesh

This comment has been minimized.

Copy link
Member

@benlesh benlesh commented Mar 15, 2018

Seems like an issue with the build/bundling adding two versions of Observable. You may want to file an issue against angular-cli.

cc @IgorMinar @hansl

@LautaroLorenz

This comment has been minimized.

Copy link

@LautaroLorenz LautaroLorenz commented Mar 18, 2018

resolved using

import { Observable } from "rxjs/Observable";

instead of

import { Observable } from "rxjs"

@benlesh

This comment has been minimized.

Copy link
Member

@benlesh benlesh commented Mar 19, 2018

The correct answer for RxJS 5.X is to just import from the proper location: rxjs/Observable

In 6.0.0 (beta) we've fixed this by only having one import location for Observable which is rxjs.

@benlesh benlesh closed this Mar 19, 2018
@lock

This comment has been minimized.

Copy link

@lock lock bot commented Jun 5, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.