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

[memoize] Feature Request: Asynchronous Variants #493

Open
HitomiTenshi opened this issue Oct 15, 2024 · 0 comments
Open

[memoize] Feature Request: Asynchronous Variants #493

HitomiTenshi opened this issue Oct 15, 2024 · 0 comments

Comments

@HitomiTenshi
Copy link

The current memoize functions cache raw Promises without evaluating them. This is an issue, because I do not want to cache rejected Promises.

Reproduction:

import { memoizeO } from '@thi.ng/memoize'

function rejectAfterDelay(ms?: number) {
  return new Promise((_, reject) => setTimeout(reject, ms))
}

async function test() {
  const memoizedAsyncFn = memoizeO(async (delay: number) => {
    console.log('executing')
    await rejectAfterDelay(delay)
  })

  try {
    await memoizedAsyncFn(2000)
  } catch {
    console.log('Failure 1')
  }

  try {
    await memoizedAsyncFn(2000)
  } catch {
    console.log('Failure 2 without delay, because the same Promise got re-evaluated')
  }
}

test()

Solution:

Make asynchronous variants for all memoization functions that skip caching when the Promise is rejected.

Reason:

Throwing an error synchronously in memoize functions causes the caching to be skipped. The same thing does not happen for rejected Promises. This is fine if reusing the same failed Promise is intentional, but I would assume that for most use cases this is not the desired outcome.

Example workaround:

I wrote a workaround async variant of memoizeO to showcase the desired behavior:

import type {NumOrString} from '@thi.ng/api'
import {memoizeO} from '@thi.ng/memoize'

type Memoize<A extends NumOrString, B> = Parameters<typeof memoizeO<A, B>>
type MemoizeFunction<A extends NumOrString, B> = Memoize<A, Promise<B>>[0]
type MemoizeCache<A extends NumOrString, B> = Memoize<A, Promise<B>>[1]

/**
 * Memoizes the result of an asynchronous function. Caching is skipped when errors are raised.
 *
 * @param fn
 * @param cache
 */
export function memoizeAsync<A extends NumOrString, B>(
  fn: MemoizeFunction<A, B>,
  cache?: MemoizeCache<A, B>,
) {
  cache = cache ?? {}
  const memoized = memoizeO(fn, cache)

  return async (x: A) => {
    try {
      return await memoized(x)
    } catch (error) {
      delete cache[x]
      throw error
    }
  }
}
postspectacular added a commit that referenced this issue Oct 31, 2024
BREAKING CHANGE: remove obsolete arity overrides (i.e. 2/3/4 suffixed versions)

- add memoizeAsync()
- add memoizeAsync1()
- add memoizeAsyncJ()
- add memoizeAsyncO()
- refactor memoize fns to be variadic
- remove obsolete fixed arity versions (e.g. `memoize2O`, 3O, 4O etc.)
- update tests
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

1 participant