jmdobry / CacheFactory

CacheFactory is a very simple and useful synchronous key-value store for the browser.
http://www.pseudobry.com/CacheFactory
MIT License
30 stars 18 forks source link

fix(promise): don't change source promise #30

Open Delagen opened 7 years ago

Delagen commented 7 years ago

Don't change promise data when caching which cause angular errors in angular-cache project

codecov-io commented 7 years ago

Codecov Report

Merging #30 into master will increase coverage by 0.27%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   91.88%   92.15%   +0.27%     
==========================================
  Files           6        6              
  Lines         702      701       -1     
==========================================
+ Hits          645      646       +1     
+ Misses         57       55       -2
Impacted Files Coverage Δ
src/Cache.js 92.19% <100%> (+0.19%) :white_check_mark:
src/BinaryHeap.js 90.9% <0%> (+1.01%) :white_check_mark:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d31b836...9f72304. Read the comment docs.

jmdobry commented 7 years ago

Can you provide a little more background on the need for this change? Tests?

Delagen commented 7 years ago

See angular-cache tests. In case promise stored in cache it also returns as promise till it resolved. Reassign v variable in this case and return breaks original data in promise chain awaiting by angular

Delagen commented 7 years ago

See this parts of angular source

if (cache) {
        cachedResp = cache.get(url);
        if (isDefined(cachedResp)) {
          if (isPromiseLike(cachedResp)) {
            // cached request has already been sent, but there is no response yet
            cachedResp.then(resolvePromiseWithResult, resolvePromiseWithResult);
          } else {
            // serving from cache
            if (isArray(cachedResp)) {
              resolvePromise(cachedResp[1], cachedResp[0], shallowCopy(cachedResp[2]), cachedResp[3]);
            } else {
              resolvePromise(cachedResp, 200, {}, 'OK');
            }
          }
        } else {
          // put the promise for the non-transformed response into cache as a placeholder
          cache.put(url, promise);
        }
      }
...
 function resolvePromiseWithResult(result) {
        resolvePromise(result.data, result.status, shallowCopy(result.headers()), result.statusText);
      }

So if cache return something and it's type is PromiseLike so it's parsed as original response (object-like) otherwise cache object is parsed as stored in cache

In case you change data in then promise pipeline it breaks on result.headers() call

Delagen commented 7 years ago

When merge please see my (comment)[https://github.com/jmdobry/angular-cache/issues/224#issuecomment-282945075] which may be fix https://github.com/jmdobry/angular-cache/issues/224

Delagen commented 7 years ago

Any news?

alfaproject commented 7 years ago

@jmdobry please? |: