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

Add handling of leading/trailing edges #283

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 48 additions & 5 deletions src/throttle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,29 @@ type EventAsReturnType<Payload> = any extends Payload ? Event<Payload> : never;
export function throttle<T>(_: {
source: Unit<T>;
timeout: number | Store<number>;
leading?: boolean | Store<boolean>;
trailing?: boolean | Store<boolean>;
name?: string;
}): EventAsReturnType<T>;
export function throttle<T, Target extends Unit<T>>(_: {
source: Unit<T>;
timeout: number | Store<number>;
target: Target;
leading?: boolean | Store<boolean>;
trailing?: boolean | Store<boolean>;
name?: string;
}): Target;
export function throttle<T>({
source,
timeout,
leading,
trailing,
target = createEvent<T>(),
}: {
source: Unit<T>;
timeout: number | Store<number>;
leading?: boolean | Store<boolean>;
trailing?: boolean | Store<boolean>;
name?: string;
target?: Unit<any>;
}): EventAsReturnType<T> {
Expand All @@ -42,13 +50,18 @@ export function throttle<T>({
);

// It's ok - nothing will ever start unless source is triggered
const $payload = createStore<T>(null as unknown as T, { serialize: 'ignore' }).on(
source,
(_, payload) => payload,
);
const $payload = createStore<T>(null as unknown as T, {
serialize: 'ignore',
}).on(source, (_, payload) => payload);

const triggerTick = createEvent<T>();

const $leading = toStoreBoolean(leading, '$leading', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

In lodash both leading and trailing are true by default. Why not make the same?

Copy link
Author

@Dema Dema Apr 27, 2023

Choose a reason for hiding this comment

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

I didn't want to change the default behavior making it breaking change, but I agree, I'd rather made them both true by default

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const $leading = toStoreBoolean(leading, '$leading', false);
const $leading = toStoreBoolean(leading ?? false, '$leading');

const $trailing = toStoreBoolean(trailing, '$trailing', true);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const $trailing = toStoreBoolean(trailing, '$trailing', true);
const $trailing = toStoreBoolean(trailing ?? true, '$trailing');


const $neverCalled = createStore(true).on(target, () => false);
Copy link

@Toleckk Toleckk May 16, 2023

Choose a reason for hiding this comment

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

This one should be reset every time when timeout finishes, I guess

const trigger = createEvent()

const throttled = throttle({ source: trigger, timeout: 100, leading: true })

trigger(1)
trigger(2)
trigger(3)
await wait(110)
trigger(4)

throttled should be called with 4 immediately

const $lastCalled = createStore<number>(0).on(target, () => Date.now());

const $canTick = createStore(true, { serialize: 'ignore' })
.on(triggerTick, () => false)
.on(target, () => true);
Expand All @@ -66,8 +79,25 @@ export function throttle<T>({
});

sample({
source: $payload,
clock: $payload,
source: [$leading, $neverCalled],
filter: ([leading, neverCalled]) => leading && neverCalled,
target,
});

sample({
source: [$trailing, $payload] as const,
clock: timerFx.done,
filter: ([trailing]) => trailing,
fn: ([_, payload]) => payload,
target,
});
sample({
clock: $payload,
source: { trailing: $trailing, lastCalled: $lastCalled, timeout: $timeout },
filter: ({ trailing, lastCalled, timeout }) =>
!trailing && lastCalled + timeout < Date.now(),
fn: (_src, clk) => clk,
target,
});

Expand All @@ -88,3 +118,16 @@ function toStoreNumber(value: number | Store<number> | unknown): Store<number> {
`timeout parameter should be number or Store. "${typeof value}" was passed`,
);
}

function toStoreBoolean(
value: boolean | Store<boolean> | undefined,
name: string,
defaultValue: boolean,
): Store<boolean> {
if (is.store(value)) return value;
if (typeof value === 'boolean') {
return createStore(value, { name });
} else {
return createStore(defaultValue, { name });
}
}
Comment on lines +122 to +133
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function toStoreBoolean(
value: boolean | Store<boolean> | undefined,
name: string,
defaultValue: boolean,
): Store<boolean> {
if (is.store(value)) return value;
if (typeof value === 'boolean') {
return createStore(value, { name });
} else {
return createStore(defaultValue, { name });
}
}
function toStoreBoolean(value: boolean | Store<boolean> | undefined, name: string): Store<boolean> {
if (is.store(value)) return value;
return createStore(value, { name });
}

10 changes: 10 additions & 0 deletions src/throttle/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ target = throttle({ source, timeout });

1. `source` ([_`Event`_] | [_`Store`_] | [_`Effect`_]) — Source unit, data from this unit used by the `target`
1. `timeout` ([_`number`_] | `Store<number>`) — time to wait before trigger `target` after last trigger or `source` trigger
1. `leading` ([_`boolean`_] | `Store<boolean>`) — trigger `target` on the leading edge of the `timeout`. If `true` first trigger of `source` causes
immediate trigger of `target`, default is `false`, `target` will be first triggered only after `timeout`
1. `trailing` ([_`boolean`_] | `Store<boolean>`) — trigger `target` on the trailing edge of the `timeout`. If `true` last trigger of `source`
within `timeout` causes trigger of `target` after `timeout` expires. If `false` any trigger of `source`
will be ignored completely within the `timeout` not causing trigger of `target` after `timeout` expires.

### Returns

Expand Down Expand Up @@ -105,6 +110,11 @@ throttle({ source, timeout, target });
1. `source` ([_`Event`_] | [_`Store`_] | [_`Effect`_]) — Source unit, data from this unit used by the `target`
1. `timeout` ([_`number`_] | `Store<number>`) — time to wait before trigger `target` after last trigger or `source` trigger
1. `target` ([_`Event`_] | [_`Store`_] | [_`Effect`_]) — Target unit, that triggered each time after triggering `source` with argument from `source`
1. `leading` ([_`boolean`_] | `Store<boolean>`) — trigger `target` on the leading edge of the `timeout`. If `true` first trigger of `source` causes
immediate trigger of `target`, default is `false`, `target` will be first triggered only after `timeout`
1. `trailing` ([_`boolean`_] | `Store<boolean>`) — trigger `target` on the trailing edge of the `timeout`. If `true` last trigger of `source`
withing `timeout` causes trigger of `target` after `timeout` expires. If `false` any trigger of `source`
will be ignored completely within the `timeout` not causing trigger of `target` after `timeout` expires.

### Returns

Expand Down