Skip to content

Conversation

@Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Feb 5, 2025

Problem

debounce is reimplemented in multiple places. There is debounce:

export function debounce<T>(cb: () => T | Promise<T>, delay: number = 0): () => Promise<T> {
let timeout: Timeout | undefined
let promise: Promise<T> | undefined
return () => {
timeout?.refresh()
return (promise ??= new Promise<T>((resolve, reject) => {
timeout = new Timeout(delay)
timeout.onCompletion(async () => {
timeout = promise = undefined
try {
resolve(await cb())
} catch (err) {
reject(err)
}
})
}))
}
}

cancellableDebounce:
export function cancellableDebounce<T, U extends any[]>(
cb: (...args: U) => T | Promise<T>,
delay: number = 0
): { promise: (...args: U) => Promise<T>; cancel: () => void } {
let timeout: Timeout | undefined
let promise: Promise<T> | undefined
const cancel = (): void => {
if (timeout) {
timeout.cancel()
timeout = undefined
promise = undefined
}
}
return {
promise: (...arg) => {
timeout?.refresh()
return (promise ??= new Promise<T>((resolve, reject) => {
timeout = new Timeout(delay)
timeout.onCompletion(async () => {
timeout = promise = undefined
try {
resolve(await cb(...arg))
} catch (err) {
reject(err)
}
})
}))
},
cancel: cancel,
}
}

and a very similar function keyedDebounce:
function keyedDebounce<T, U extends any[], K extends string = string>(
fn: (key: K, ...args: U) => Promise<T>
): typeof fn {
const pending = new Map<K, Promise<T>>()
return (key, ...args) => {
if (pending.has(key)) {
return pending.get(key)!
}
const promise = fn(key, ...args).finally(() => pending.delete(key))
pending.set(key, promise)
return promise
}
}

These functions should share common implementation logic, and live in the same place for easier discoverability.

Solution

  • reimplement debounce in terms of cancellableDebounce since its a special case.
  • move these functions to shared, common location.
  • augment debounce to allow the callback to take arguments.

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

github-actions bot commented Feb 5, 2025

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@Hweinstock Hweinstock changed the title refactor(debounce): avoid reimplementations of the same logic. (WIP) refactor(debounce): avoid reimplementations of the same logic. Feb 5, 2025
@Hweinstock Hweinstock marked this pull request as ready for review February 5, 2025 19:14
@Hweinstock Hweinstock requested a review from a team as a code owner February 5, 2025 19:14
@Hweinstock Hweinstock merged commit 928593c into aws:master Feb 7, 2025
26 checks passed
@Hweinstock Hweinstock deleted the refactor/debounce branch February 7, 2025 13:24
s7ab059789 pushed a commit to s7ab059789/aws-toolkit-vscode that referenced this pull request Feb 19, 2025
)

## Problem
`debounce` is reimplemented in multiple places. There is `debounce`:
https://github.com/aws/aws-toolkit-vscode/blob/100eb0737789d9d5ba4b04e055730b467bd97e14/packages/core/src/shared/utilities/functionUtils.ts#L89-L108
`cancellableDebounce`:
https://github.com/aws/aws-toolkit-vscode/blob/100eb0737789d9d5ba4b04e055730b467bd97e14/packages/core/src/shared/utilities/functionUtils.ts#L114-L147
and a very similar function `keyedDebounce`:
https://github.com/aws/aws-toolkit-vscode/blob/100eb0737789d9d5ba4b04e055730b467bd97e14/packages/core/src/auth/auth.ts#L107-L122

These functions should share common implementation logic, and live in
the same place for easier discoverability.

## Solution
- reimplement `debounce` in terms of `cancellableDebounce` since its a
special case.
- move these functions to shared, common location. 
- augment `debounce` to allow the callback to take arguments. 

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
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

Successfully merging this pull request may close these issues.

2 participants