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

finalize not working as expected with streams from dependencies #5237

Closed
cdanielw opened this issue Jan 16, 2020 · 4 comments
Closed

finalize not working as expected with streams from dependencies #5237

cdanielw opened this issue Jan 16, 2020 · 4 comments
Labels
bug Confirmed bug

Comments

@cdanielw
Copy link

Bug Report

Current Behavior
Given project a and b. When a subscribe to a stream from b, the finalize operator doesn't get called.

The below script outputs:

a finalized c

Reproduction
https://github.com/cdanielw/rxjs-finalize

const {finalize, first} = require('rxjs/operators')
const anotherModule$ = require('b')
const sameModule$ = require('./c')

anotherModule$.pipe(
        // Stream from another module doesn't finalize
	finalize(() => console.log('a finalized b')),
	first()
).subscribe()

sameModule$.pipe(
        // Stream from same module finalize
	finalize(() => console.log('a finalized c')),
	first()
).subscribe()

Expected behavior
Script should output :

a finalized b
a finalized c

Environment

  • Runtime: Node v10.14.2
  • RxJS version: 6.5.4
@cdanielw
Copy link
Author

cdanielw commented Jan 16, 2020

I managed to workaround it:

of(true).pipe(
    switchMap(() => anotherModule$),
    finalize(() => console.log('1. a finalized b')),
    first()
).subscribe()

But it's easy to forget to do this, leading to subtle bugs.

@cartant
Copy link
Collaborator

cartant commented Jan 16, 2020

from(anotherModule$) should be used. I'll write an explanation when I'm less tired. Not a bug, IMO.

@cdanielw
Copy link
Author

Having different code depending on how/where code has been required makes it very difficult to pull RxJS code into shared libraries. This is where my issue is coming from.

Even if this wouldn't be considered a bug, I'd really like you to consider removing any differences in behaviour based on where streams/code creating streams have been required from.

@cartant
Copy link
Collaborator

cartant commented Jan 16, 2020

Yeah. I was wrong; this is definitely a bug.

I guess I really was tired, last night, as I recently fixed a problem that was closely related to this - see #5059 and the associated cherry pick into v6.

That PR includes testing infrastructure that'll allow me to reproduce your problem in a failing test. After which, I'll fix it.

Thanks for opening the issue.

@cartant cartant added the bug Confirmed bug label Jan 16, 2020
cartant added a commit to cartant/rxjs that referenced this issue Jan 17, 2020
cartant added a commit to cartant/rxjs that referenced this issue Jan 17, 2020
cartant added a commit to cartant/rxjs that referenced this issue Jan 17, 2020
cartant added a commit to cartant/rxjs that referenced this issue Jan 17, 2020
cartant added a commit to cartant/rxjs that referenced this issue Jan 17, 2020
cartant added a commit to cartant/rxjs that referenced this issue Apr 23, 2020
cartant added a commit to cartant/rxjs that referenced this issue Apr 23, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants