cujojs / most

Ultra-high performance reactive programming
MIT License
3.49k stars 231 forks source link

fromPromise is not disposed correctly #534

Closed marcus13371337 closed 4 years ago

marcus13371337 commented 4 years ago

Summary

Creating a stream with fromPromise and then unsubscribing to it, most will execute the sequential steps even though the stream should be disposed

As example:

    most
      .fromPromise(new Promise(resolve => setTimeout(resolve, 2000)))
      .tap(() => console.log('1'))
      .merge(most.just().delay(1500)) // since 1500 < 2000, the fromPromise-stream should be unsubscribed at 1500 and any result after that should be ignored
      .take(1)
      .subscribe({
        next: () => console.log('Finished'),
      })

Expected result

console.log('Finished')

Actual Result

console.log('Finished') console.log('1')

Versions

Seems to have been introduced when the following refactor was done: https://gist.github.com/briancavalier/a27ddc247dd118ae5508

Steps to reproduce

See above

Code to reproduce

    most
      .fromPromise(new Promise(resolve => setTimeout(resolve, 2000)))
      .tap(() => console.log('1'))
      .merge(most.just().delay(1500)) // since 1500 < 2000, the fromPromise-stream should be unsubscribed at 1500 and any result after that should be ignored
      .take(1)
      .subscribe({
        next: () => console.log('Finished'),
      })
briancavalier commented 4 years ago

Hey @marcus13371337, thanks for the detailed report and example. I just opened a candidate fix in https://github.com/cujojs/most/pull/535. Feel free to try that branch to see if it solves the issue for you.

briancavalier commented 4 years ago

@marcus13371337 We published 1.8.1 with the fix.

mikeduminy commented 4 years ago

Thanks @briancavalier and thanks @marcus13371337 for the report

mikeduminy commented 4 years ago

@briancavalier the changelog in the releases on github hasn't been updated since 1.7.2 What changed in 1.8.0?

marcus13371337 commented 4 years ago

Thank you so much!