Leading/Trailing throttle and throttleTime configuration #2465
Conversation
Adds the ability to configure the `throttle` operator via a
configuration object in the same manner as Lodash's throttle.
Currently this defaults to the existing behavior, which is `{ leading:
true, trailing: false }`. In an upcoming major version, I think we
should change this to align with other libraries that have similar
functionality. In particular Lodash, given it's popularity.
NOTE: While working on this I discovered that the leading value is
actually being emitted *after* the durationSelector is called and
subscribed to. This is likely a bug, but I'll file a separate issue.
related #1625
spec/operators/throttleTime-spec.ts
Outdated
| }); | ||
|
|
||
| describe('throttleTime(fn, { leading: false, trailing: true })', () => { | ||
| asDiagram('throttleTime(fn, { leading: true, trailing: true })')('should immediately emit the first value in each time window', () => { |
There was a problem hiding this comment.
{ leading: true, trailing: true } should be { leading: false, trailing: true }
spec/operators/throttle-spec.ts
Outdated
| }); | ||
|
|
||
| describe('throttle(fn, { leading: false, trailing: true })', () => { | ||
| asDiagram('throttle(fn, { leading: true, trailing: true })')('should immediately emit the first value in each time window', () => { |
There was a problem hiding this comment.
{ leading: true, trailing: true } should be { leading: false, trailing: true }
|
Is it fair to say...
Also would it be possible (perhaps as a separate PR!) to give the same treatment to We have no operator that compares to lodash's |
That's the idea. |
|
@david-driscoll the changes are made. Good catch. I put them in a separate commit so I didn't have to pull everything apart and change two commits. |
|
Looks good to me. @kwonoj @trxcllnt @jayphelps any complaints? |
|
The potential bug found in |
|
Hi, I would be interested in knowing when you guys think this will be in a release? We have a workaround that we can remove once this goes in and I was wondering when you think this will be merged. Thanks |
|
I could also do with this change for my project. Any update would be greatly appreciated. |
|
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. |
related to #1625 see commits for details.