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

ThrottleTime with immediate output but all events must be n ms apart #5574

Open
trungphan opened this issue Jul 5, 2020 · 3 comments
Open
Assignees

Comments

@trungphan
Copy link

trungphan commented Jul 5, 2020

Feature Request

Is your feature request related to a problem? Please describe.
With the current throttleTime behavior with config {leading: true, trailing: true}, events are not guaranteed n ms apart. For example:

const observable = cold('-abcdef-----abcdef|');
const throttled = observable.pipe(
    throttleTime(time('---|'), getTestScheduler(), {leading: true, trailing: true}));
getTestScheduler().expectObservable(throttled).toBe('-a--de--f---a--de-(f|)');

Describe the solution you'd like
I want all events to be at least n ms apart. Specifically:

const observable = cold('-abcdef-----abcdef|');
const throttled = observable.pipe(
    throttleTime(time('---|'), getTestScheduler(), someFooConfig));
getTestScheduler().expectObservable(throttled).toBe('-a--d--f----a--d--(f|)');

Describe alternatives you've considered
I'm currently using throttleTime with {leading: true, trailing: true}, but that's not ideal since my RPCs are not always n ms apart (causing them being throttled from the server). The default config {leading: true, trailing: false} does not cut it because some trailing events are missing. The config {leading: false, trailing: true} does not cut it either because I'd like to process the event immediately if possible.

(If this is new operator request) describe reason it should be core operator
This can be achieved in the user-land module by just clone the throttleTime code and does the fix myself in my code, but that's not very elegant.

Additional context
This feature seems to be the one requested in #1625 , but the fix did not achieve that objective.

@benlesh benlesh added the AGENDA ITEM Flagged for discussion at core team meetings label Jul 8, 2020
@benlesh
Copy link
Member

benlesh commented Jul 8, 2020

I can't tell if this is a bug with the current behavior, or it should be a new feature. IMO, something isn't "throttled" if two events can be fired in less time than the throttle. 🤔

@benlesh
Copy link
Member

benlesh commented Jul 29, 2020

Discussed with the team... we will try to align this with whatever lodash is doing.

@benlesh benlesh self-assigned this Jul 29, 2020
@benlesh benlesh removed the AGENDA ITEM Flagged for discussion at core team meetings label Jul 29, 2020
@cartant
Copy link
Collaborator

cartant commented May 1, 2021

TBH, this seems related to #4429 - i.e. inspectTime is, AFAICT, what's being sought, here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants