Bring back a hook that is called after an item expires

[vlad-zhukov]

v4 introduced a breaking change that is not mentioned in the changelog – onExpire is now called before an item will be removed from the cache.

const onExpire = (key) => {
  console.log(moized.has([key])) // => false in 3.5, but true in 4.0
}

I would like to have a hook that is called after an item was removed, that was my original idea behind onExpire. I can see that onExpire might be useful in it's current form and I am not proposing to revert the changes, but I would like to discuss other options to bring this feature back.

For now I've got 2 ideas:

• add a new callback afterExpire (easy to implement)
• return a Promise from onExpire and resolve it when an item was removed (not sure how to make it right and how would it look like for a consumer's perspective)

Sticking with 3.5 for now.

[planttheidea]

Hmm ... yes I see what you are talking about, since we have the potential of programmatically canceling the expire operation, we have reversed the chicken with the egg in the order of operations and can be misleading. This was not the intent of the change, but certainly was an unforeseen consequence of it.

I can tell you straight up that I don't like the idea of adding another callback, nor do I like the promise idea (which would be more elegant), because both could easily introduce some very weird complexity issues for an edge case. My original thought may sound a bit weird, but think on it a second after a gut reaction ... the re-initialization of an item post-expiration if onExpire returns false.

    const timeoutId = setTimeout(() => {
      const value = this.get(key);

      this.remove(key);
      this.expirations.splice(findExpirationIndex(this.expirations, key), 1);

      // $FlowIgnore onExpire will only fire if it is a function
      if (isFunction(onExpire) && onExpire(key.key) === false) {
        this.add(key, value);

        return this.expireAfter(key);
      }
    }, maxAge);

This ensures order of operations are returned to the expected behavior (removal happens prior to onExpire firing), and also maintains the intent of the original change (if we programmatically cancel the expiration, the cached value is still present and the the expiration is reset). What do you think? I modded this change locally and it appears to work as described, but honestly it was a 30-second smoke test ... if you think this will satisfy what you are looking for then I can give a more thorough testing on it.

[vlad-zhukov]

I am fine with this! 👍 I had a thought with this approach too but I think I filtered it away very fast because I thought you wouldn't like it for some reason 😄

Do you need any help with this?

[planttheidea]

Nope, all good ... I'll report back if it turns out my deeper dive shows it doesn't work as expected, otherwise expect a release for this fix very soon.

[planttheidea]

@vlad-zhukov if you want to check out my PR for the changes, let me know what you think. I had to make some small tweaks compared to the POC code I showed you prior, but its the same concept just a more accurate implementation (it also accounts for the very remote possibility that someone manually removed the key from cache prior to the expiration, which it did not before). Also mind the noise ... the additional files are from running prepublish:compile and updating the docs, plus some quirkiness in using the new flow-bin ... the real file changes here are just in src/Cache.js.

[planttheidea]

Fixed in 4.0.1

[vlad-zhukov]

Thank you! 🎉