pamelafox / lscache

A localStorage-based memcache-inspired client-side caching library.
Other
1.46k stars 160 forks source link

flush() function fails to remove all cached items #99

Closed tcchau closed 2 years ago

tcchau commented 2 years ago

At least on Chrome, the order that the keys are returned to the caller may change if entries in localStorage are removed. Therefore, the logic of using an indexer to scan the entries in the cache is not guaranteed to exhaustively iterate over all keys.

function eachKey(fn) {
    var prefixRegExp = new RegExp('^' + CACHE_PREFIX + escapeRegExpSpecialCharacters(cacheBucket) + '(.*)');
    // Loop in reverse as removing items will change indices of tail
    for (var i = localStorage.length-1; i >= 0 ; --i) {
      var key = localStorage.key(i);
      key = key && key.match(prefixRegExp);
      key = key && key[1];
      if (key && key.indexOf(CACHE_SUFFIX) < 0) {
        fn(key, expirationKey(key));
      }
    }
  }

A safer implementation is to iterate over localStorage without mutating it to create a list of keys to be removed, and then remove them as a second step.

pamelafox commented 2 years ago

This is the same issue addressed by https://github.com/pamelafox/lscache/pull/96 I believe ? I didn't merge that yet as there was some debate as to the best code, feel free to take a look and comment. Perhaps I'll have time to merge it and make a released during the holidays.

tcchau commented 2 years ago

This is the same issue addressed by #96 I believe ? I didn't merge that yet as there was some debate as to the best code, feel free to take a look and comment. Perhaps I'll have time to merge it and make a released during the holidays.

Yes, the code in the fix for #96 should work. It's the same approach I'm using in my fork. Sorry I didn't catch it as my brief search of the issues didn't seem to match anything.

pamelafox commented 2 years ago

Sorry for delay, lscache on npm now has the fix!