Dispose the source disposable #472
Conversation
When disposing a debounce stream we also need to dispose the source disposable
src/combinator/limit.js
Outdated
| @@ -110,6 +110,7 @@ DebounceSink.prototype.error = function (t, x) { | |||
|
|
|||
| DebounceSink.prototype.dispose = function () { | |||
| this._clearTimer() | |||
| this.disposable.dispose() | |||
There was a problem hiding this comment.
Yikes. Good catch, @ppoliani. This just needs a return, return this.disposable.dispose() since disposables typically form a promise chain in the current version of most.js.
|
It'd be great to add a regression test for this as well. There's a FakeDisposeSource in test/helper that is helpful in writing tests to verify disposal, and here's an example of using it (there are other examples in the test folder as well). |
|
Looking good @ppoliani. Thanks for adding the test. I was porting your fix to Take a look at mostjs/core#107 and see what you think. If that seems right, then you can do the same here. |
|
@briancavalier The change makes sense 😄 I'll include it here, as well. |
|
Awesome, thank you, @ppoliani. We'll get this out in the next release soon. |
When disposing a debounce stream we also need to dispose the source disposable
Summary
I was trying to use
debouncein the following scenarioBut I soon realized that when I tried to dispose the above stream, it would ignore disposing the stream (source stream) that comes before the debounce.
Todo