medikoo / memoizee

Complete memoize/cache solution for JavaScript
ISC License
1.74k stars 61 forks source link

Introduce 'promise' mode #35

Closed medikoo closed 8 years ago

medikoo commented 9 years ago

At this point promise returning functions are memoized natural way, which seems ok, but there's one flaw, Rejected promise results are treated same as successful ones, that's not right.

We should not store them, same way as we do not store sync function crashes, or errors in async mode.

theothermattm commented 9 years ago

+1 could definitely use this!

asaf-romano commented 8 years ago

+1.

Here's a temporary workaround I'm using (arity is a local helper that sets the length of a function):

function doMemoizeFunction(func, options)
{
    let memoized = memoize(arity(options.length || func.length,
                                 function(...args)
                                 {
                                     let result = func.apply(this, args);
                                     if (result && typeof result.then == "function")
                                     {
                                         result.then(null, () => memoized.delete(...args));
                                     }
                                     return result;
                                 }), options);
    return memoized;
}
medikoo commented 8 years ago

Other workaround, can look as:

var memoized = memoize(function () {
  var context = this, args = arguments;
  return returnsPromise.apply(context, args).then(null, function (err) {
    memoized.delete.apply(context, args);
    throw err;
  });
}, { length: returnsPromise.length });

Still it's not ideal when combined with other options as maxAge, dispose etc.

puzrin commented 8 years ago

I have a couple of strange question:

medikoo commented 8 years ago

@puzrin indeed your questions are not clear to me. Answering best I could:

puzrin commented 8 years ago

I considered, does it worth to create separate package for promises with my own efforts. Seems, that will have almost no advantages (+ additional time spends for maintenance).

medikoo commented 8 years ago

@puzrin you can introduce external extensions, e.g. that way:

// Configure new extension
var myExtension = function (myExtensionOptions, memoizedFunctionData, memoizeOptions) {
 ...
};
require('memoizee/lib/registered-extensions').myExtension = myExtension;

// Memoize function and use provided extension
var fn = function (...) {...}
var memoizedFn = memoize(fn, { myExtension: myExtensionOptions })

Just be sure to not use promise name so it won't collide with one that will soon be added.

puzrin commented 8 years ago

https://github.com/nodeca/promise-memoize did a separate simple package. No weakmaps, no lru, no stat... but it works :). Also does not drop cache on prefetch and unref timers.

medikoo commented 8 years ago

https://github.com/nodeca/promise-memoize did a separate simple package.

Great if that works for you, and sorry that I couldn't handle that one earlier

medikoo commented 8 years ago

Published with v0.4.0