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.

Sign up for GitHub

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

Jump to bottom

fix(throttle): properly handle default ThrottleConfig values #7176

Merged
merged 2 commits into from Apr 6, 2023
Merged

fix(throttle): properly handle default ThrottleConfig values #7176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c8a649
fix(throttle): properly handle default ThrottleConfig values
jakovljevic-mladen Jan 31, 2023
f9cdf18
docs(ThrottleConfig): document "leading" and "trailing" behaviour
jakovljevic-mladen Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions spec/operators/throttle-spec.ts
View file
Open in desktop
Expand Up @@ -398,7 +398,7 @@ describe('throttle', () => {
];
const expected = '-a---y----b---x---x---x---|';

const result = e1.pipe(throttle(() => e2, { leading: true, trailing: true }));
const result = e1.pipe(throttle(() => e2, { trailing: true }));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
Expand All @@ -414,7 +414,7 @@ describe('throttle', () => {
const n1Subs = ['--^------------------! '];
const exp = ' --x------------------| ';

const result = s1.pipe(throttle(() => n1, { leading: true, trailing: true }));
const result = s1.pipe(throttle(() => n1, { trailing: true }));
expectObservable(result).toBe(exp);
expectSubscriptions(s1.subscriptions).toBe(s1Subs);
expectSubscriptions(n1.subscriptions).toBe(n1Subs);
Expand All @@ -433,7 +433,7 @@ describe('throttle', () => {
];
const expected = '-a--------x---(y|)';

const result = e1.pipe(throttle(() => e2, { leading: true, trailing: true }));
const result = e1.pipe(throttle(() => e2, { trailing: true }));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
Expand Down
42 changes: 21 additions & 21 deletions spec/operators/throttleTime-spec.ts
View file
Open in desktop
Expand Up @@ -20,7 +20,7 @@ describe('throttleTime operator', () => {
const expected = '-a--------b-----c----|';
const subs = ' ^--------------------!';

const result = e1.pipe(throttleTime(5, rxTest));
const result = e1.pipe(throttleTime(5));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
Expand All @@ -31,7 +31,7 @@ describe('throttleTime operator', () => {
of(1, 2, 3)
.pipe(throttleTime(5))
.subscribe({
next: (x: number) => {
next: (x) => {
expect(x).to.equal(1);
},
complete: done,
Expand All @@ -41,17 +41,17 @@ describe('throttleTime operator', () => {
it('should throttle events multiple times', () => {
const expected = ['1-0', '2-0'];
concat(
timer(0, 1, rxTest).pipe(
timer(0, 1).pipe(
take(3),
map((x: number) => '1-' + x)
map((x) => '1-' + x)
),
timer(8, 1, rxTest).pipe(
timer(8, 1).pipe(
take(5),
map((x: number) => '2-' + x)
map((x) => '2-' + x)
)
)
.pipe(throttleTime(5, rxTest))
.subscribe((x: string) => {
.subscribe((x) => {
expect(x).to.equal(expected.shift());
});

Expand All @@ -64,7 +64,7 @@ describe('throttleTime operator', () => {
const subs = ' ^--------------------!';
const expected = '-a--------b-----c----|';

expectObservable(e1.pipe(throttleTime(5, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(5))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -75,7 +75,7 @@ describe('throttleTime operator', () => {
const subs = ' ^------------------------!';
const expected = 'a-----a-----a-----a-----a|';

expectObservable(e1.pipe(throttleTime(5, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(5))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -86,7 +86,7 @@ describe('throttleTime operator', () => {
const subs = ' ^----!';
const expected = '-----|';

expectObservable(e1.pipe(throttleTime(5, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(5))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -97,7 +97,7 @@ describe('throttleTime operator', () => {
const subs = ' ^----!';
const expected = '-----#';

expectObservable(e1.pipe(throttleTime(10, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(10))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -108,7 +108,7 @@ describe('throttleTime operator', () => {
const subs = ' (^!)';
const expected = '|';

expectObservable(e1.pipe(throttleTime(30, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(30))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -119,7 +119,7 @@ describe('throttleTime operator', () => {
const subs = ' ^';
const expected = '-';

expectObservable(e1.pipe(throttleTime(30, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(30))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -130,7 +130,7 @@ describe('throttleTime operator', () => {
const subs = ' (^!)';
const expected = '#';

expectObservable(e1.pipe(throttleTime(30, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(30))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -142,7 +142,7 @@ describe('throttleTime operator', () => {
const subs = ' ^------------------------------!';
const expected = '-a-------------d----------------';

expectObservable(e1.pipe(throttleTime(5, rxTest)), unsub).toBe(expected);
expectObservable(e1.pipe(throttleTime(5)), unsub).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -155,9 +155,9 @@ describe('throttleTime operator', () => {
const unsub = ' -------------------------------!';

const result = e1.pipe(
mergeMap((x: string) => of(x)),
throttleTime(5, rxTest),
mergeMap((x: string) => of(x))
mergeMap((x) => of(x)),
throttleTime(5),
mergeMap((x) => of(x))
);

expectObservable(result, unsub).toBe(expected);
Expand All @@ -171,7 +171,7 @@ describe('throttleTime operator', () => {
const subs = ' ^------------------------------!';
const expected = '-a-------------d---------------#';

expectObservable(e1.pipe(throttleTime(5, rxTest))).toBe(expected);
expectObservable(e1.pipe(throttleTime(5))).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(subs);
});
});
Expand All @@ -186,7 +186,7 @@ describe('throttleTime operator', () => {
// ----|----|---|---|
const expected = '-a---y----b---x---x---(x|)';

const result = e1.pipe(throttleTime(t, rxTest, { leading: true, trailing: true }));
const result = e1.pipe(throttleTime(t, rxTest, { trailing: true }));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
Expand All @@ -199,7 +199,7 @@ describe('throttleTime operator', () => {
const t = time(' ----| ');
const expected = '-a--------------------|';

const result = e1.pipe(throttleTime(t, rxTest, { leading: true, trailing: true }));
const result = e1.pipe(throttleTime(t, rxTest, { trailing: true }));

expectObservable(result).toBe(expected);
});
Expand Down
47 changes: 32 additions & 15 deletions src/internal/operators/throttle.ts
View file
Open in desktop
Expand Up @@ -5,16 +5,36 @@ import { operate } from '../util/lift';
import { createOperatorSubscriber } from './OperatorSubscriber';
import { innerFrom } from '../observable/innerFrom';

/**
* An object interface used by {@link throttle} or {@link throttleTime} that ensure
* configuration options of these operators.
*
* @see {@link throttle}
* @see {@link throttleTime}
*/
export interface ThrottleConfig {
/**
* If `true`, the resulting Observable will emit the first value from the source
* Observable at the **start** of the "throttling" process (when starting an
* internal timer that prevents other emissions from the source to pass through).
* If `false`, it will not emit the first value from the source Observable at the
* start of the "throttling" process.
*
* If not provided, defaults to: `true`.
*/
leading?: boolean;
/**
* If `true`, the resulting Observable will emit the last value from the source
* Observable at the **end** of the "throttling" process (when ending an internal
* timer that prevents other emissions from the source to pass through).
* If `false`, it will not emit the last value from the source Observable at the
* end of the "throttling" process.
*
* If not provided, defaults to: `false`.
*/
trailing?: boolean;
}

export const defaultThrottleConfig: ThrottleConfig = {
leading: true,
trailing: false,
};

/**
* Emits a value from the source Observable, then ignores subsequent source
* values for a duration determined by another Observable, then repeats this
Expand Down Expand Up @@ -53,20 +73,17 @@ export const defaultThrottleConfig: ThrottleConfig = {
* @see {@link sample}
* @see {@link throttleTime}
*
* @param durationSelector A function
* that receives a value from the source Observable, for computing the silencing
* duration for each source value, returned as an Observable or a Promise.
* @param config a configuration object to define `leading` and `trailing` behavior. Defaults
* to `{ leading: true, trailing: false }`.
* @param durationSelector A function that receives a value from the source
* Observable, for computing the silencing duration for each source value,
* returned as an `ObservableInput`.
* @param config A configuration object to define `leading` and `trailing`
* behavior. Defaults to `{ leading: true, trailing: false }`.
* @return A function that returns an Observable that performs the throttle
* operation to limit the rate of emissions from the source.
*/
export function throttle<T>(
durationSelector: (value: T) => ObservableInput<any>,
config: ThrottleConfig = defaultThrottleConfig
): MonoTypeOperatorFunction<T> {
export function throttle<T>(durationSelector: (value: T) => ObservableInput<any>, config?: ThrottleConfig): MonoTypeOperatorFunction<T> {
return operate((source, subscriber) => {
const { leading, trailing } = config;
const { leading = true, trailing = false } = config ?? {};
let hasValue = false;
let sendValue: T | null = null;
let throttled: Subscription | null = null;
Expand Down
6 changes: 3 additions & 3 deletions src/internal/operators/throttleTime.ts
View file
Open in desktop
@@ -1,5 +1,5 @@
import { asyncScheduler } from '../scheduler/async';
import { defaultThrottleConfig, throttle } from './throttle';
import { throttle, ThrottleConfig } from './throttle';
import { MonoTypeOperatorFunction, SchedulerLike } from '../types';
import { timer } from '../observable/timer';

Expand Down Expand Up @@ -47,15 +47,15 @@ import { timer } from '../observable/timer';
* internally by the optional `scheduler`.
* @param scheduler The {@link SchedulerLike} to use for
* managing the timers that handle the throttling. Defaults to {@link asyncScheduler}.
* @param config a configuration object to define `leading` and
* @param config A configuration object to define `leading` and
* `trailing` behavior. Defaults to `{ leading: true, trailing: false }`.
* @return A function that returns an Observable that performs the throttle
* operation to limit the rate of emissions from the source.
*/
export function throttleTime<T>(
duration: number,
scheduler: SchedulerLike = asyncScheduler,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should throttleTime merge scheduler and config parameters effectively introducing a ThrottleTimeConfig interface which would extend ThrottleConfig interface by adding scheduler?: SchedulerLike property to it?

It would effectively remove the need to use throttleTime like this: throttleTime(1000, undefined, { trailing: true }) to throttleTime(1000, { trailing: true }).

@benlesh, thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should probably be throttleTime(number, config) where config has a scheduler option. We'll need to deprecate the other signatures that take a scheduler as the second argument, but allow the scheduler to be passed with the config.

config = defaultThrottleConfig
config?: ThrottleConfig
): MonoTypeOperatorFunction<T> {
const duration$ = timer(duration, scheduler);
return throttle(() => duration$, config);
Expand Down