Add Promise support

[abritinthebay]

Working on a version of this on my fork now but it would be great to be able to pass in a promise and have the library return a promise that resolved to that result.

ie:

var foo = memoize(myPromiseReturningFunc);

foo(a, b, c).then((data) => {
    // do something with the data...
});

Per JS spec the official way of doing this check would be typeof fn.then == "function" as that makes the function a "thenable".

Thoughts, concerns, etc? I may open a PR if I get it in fast enough ;)

[caiogondim]

That is a brilliant idea!

As mentioned on #17 I guess we should return a specific type of memoized function based on hints the user gave us

const foo = memoize(bar, {type: 'promise'})

Due to JS lack of types, we cannot know a function will return a promise before running it

[caiogondim]

BTW, let's work together on the next major version =)

Will create a document and share with you
Once we are happy with the architecture, we can start coding

The main idea is to give a function with as less checking as possible based on hints given by user
If user don't give any hint, we have to use a generic function

[abritinthebay]

A couple of libraries basically do an options object where {promise: true} but based on the JS spec for Promise it's literally an object that has a then function as part of it.

So the test - per JS spec - would be as simple as:

if ( typeof foo === "object" && typeof foo.then === "function") {
 // it's a "thenable" which is what Promises are.
}

That's literally how V8 and other browsers do it internally too.

[caiogondim]

Yes, but this is a check on the returned value
The above code tests the return value after executing the function

But how to know (in JS) if a function returns or not a promise before executing it?

[abritinthebay]

Oh I see what you mean.

Well surely, from a memoization standpoint, it's the result that matters? So really as long as it returns the actual result (the promise) it'll work?

like... if fn(1,2,3) returns a Promise just cache that as the result and return it? Might already work tbh - not tried it ;) If not I imagine it'll be fine.

Promises are sort of their own caching - it'll always return it's data. The issue would be catching an error and not caching that.

[caiogondim]

If the function always returns a promise, we have to return a resolved promised from cache

return Promise.resolve(42)

And not

return 42

There is one more path we have to handle: we should not cache errors
Only resolved promises

[abritinthebay]

Well if the function returns a promise we can just cache that result either way. The resolved promise will always return the same data (.then on a resolved promise will always fire correctly) and we could do a quick check the first time to see if it's a promise then attach a catch that both removes the result from the cache and then throws the error to pass it down the chain.

Seems... reasonably simple?

[abritinthebay]

Pseudo Code:

const result = fn(...args);
addResultToCache(key, result);
if ( typeof result === "object" && typeof result.then === "function") {
    // it's a "thenable"
    result.catch((err) => {
        removeFromCache(key);
        throw err;
    });
}
return result;

[caiogondim]

Well if the function returns a promise we can just cache that result either way.

We can not make checks on the hot path
That would make the lib slower
Since there are no types in JS, the user must tell us beforehand about the value that function returns

It is simple and doable
But the main goal of the library is speed
So we are choosing speed over easy of use (DX)

[abritinthebay]

Right, I get that but, that shouldn't be a hot path...? It should only be called once - on the initial setting of data. It's the same number of checks either way on the set too ("do I have an options object and is it set to be a promise?" vs "is the returned data an object and does it have a then property that's a function?" In fact they both short circuit fast too (if the first check fails then no need for the second).

So the same number of checks fire either way, and is only called once. Not sure how it's a hot path or would slow anything down (any more than the alternative anyhow).

[caiogondim]

When would you do that check?

is the returned data an object and does it have a then property that's a function?

You have to do it on the returned memoized function, right?
If that is true, you would have to do it whenever the function is executed

Or am I missing something?

[abritinthebay]

You'd do it on the initial set, basically on the returned data but NOT every time, just at the point where you cache the response.

So it would only happen once.

Subsequent calls would just be a simple get from cache because no matter what you get at that point it's the (correct, non-errored) result of the cache. If it's a promise then the data is already cached in that promise (and, per pseudo code above, we don't cache errors).

You'd only need to re-memoize (and so run the check again) in the case of cache expiry (which isn't supported atm, pending that PR, and would have to be reworked anyhow).

[AlexGustafsson]

Any update on support for promises?

[caiogondim]

It's on my backlog.
Pull requests are always welcome as well.

[VanessaChu]

any update on this? is this in the latest?

[abritinthebay]

I ended up having to abandon this (for unrelated reasons) so my fork is woefully out of date, no idea if this ever landed.

[JasonKleban]

Here's a workaround with the current library (in typescript). Half to help out and half for peer review in case I missed something. The serializer seems to assume a string hash, but I needed access to the arguments for the retry case, so it requires a little bit of a hack.

import stableStringify from "fast-json-stable-stringify";
import memoize from "fast-memoize";

// A bit of a hack
interface CacheArgsAndHash { args: any[], hash: string }

// Since has/get/set are unavoidably synchronous, and since 
// there is no synchronous test for Promise state, we leverage 
// the returned promise to perform the test and retry if necessary.  
// This also lets multiple initial attempts during the pending 
// phase to share the result.  Rejected attempts are not retried, 
// but later attempts will see the rejection and attempt the cache *that*.
class PromiseAwareMemoizingCache<T extends (...args: any[]) => any> {
  private store: { [key : string]: any } = {};

  constructor(private fn : T) { }

  has = (argsAndHash : CacheArgsAndHash) => {
    return argsAndHash.hash in this.store;
  }

  get = (argsAndHash : CacheArgsAndHash) => {
    const cached = this.store[argsAndHash.hash];

    if (typeof cached === "object" && typeof cached.then === "function") { // Test for a promise
      let handled = false;

      // a race between a first possibly-pending Promise
      // and a certainly-resolved second Promise.  (First
      // position does get precedence)
      const cachedOrRetry = Promise.race([
        // if `cached` is immediately resolved, it will
        // win the race and be what is returned by this
        // invocation of `get()`.  If it is pending but
        // ultimately resolves, it will be ignored here
        // because it will already have been returned by
        // the second position logic.
        cached.catch(() => {
          // If `cached` is *immediately* rejected (it
          // is the cached promise from a previous
          // call which ultimately rejected) then
          // store and return a retry of the same call
          // instead of the cached rejection.  But if
          // `cached` was immediately pending, then
          // `handled` would be set and we should
          // ignore this ultimate late rejection.
          if (!handled) {
            const retry = this.store[argsAndHash.hash] = this.fn(...argsAndHash.args);
            return retry;
          }
        }),
        Promise.resolve().then(() => {
          // If this second position Promise wins the
          // race, it means the first was pending.
          // This handler will be run either way, but
          // the returned value will be ignored if we lost.
          // Assuming we won and the cached promise is
          // pending, mark this particular `get()`
          // invocation handled so that if it
          // ultimately fails we don't retry the
          // request.
          handled = true;
          // return the cached promise which will only
          // be returned by this invocation of `get()`
          // if it is pending.
          return cached;
        })
      ]);
      return cachedOrRetry;
    } else { // `undefined` (not found), or some other non-Promise memoized value
      return cached;
    }
  }

  set = (argsAndHash : CacheArgsAndHash, value : any) => {
    this.store[argsAndHash.hash] = value;
  }
}

// Memoizes a fetcher function (not necessarily production-ready)
export const memoizeAsync = <T extends (...args: any[]) => any>(fn : T) : T => {
  return memoize<T>(
    fn,
    {
      // The typescript types, at least, expect a string.  
      // Hacking it here, because we need the original args.
      // We could deserialize the stringified args, 
      // but this seems a little nicer.
      serializer: args => ({ args, hash: stableStringify(args) } as any), //hack
      cache: {
        create: () => new PromiseAwareMemoizingCache<T>(fn)
      } as any // Something wrong with the typescript types here related to cache.create()
    }
  );
};