<keep-alive> doesn't `destroy` cached components after when max is 1

[kchung]

Version

2.6.10

Reproduction link

https://jsfiddle.net/p0te9vL3/

Steps to reproduce

1. Open dev tools to see console
2. Cycle through the routes
3. View console and observe lifecycle hook logging

What is expected?

I'd expect once the max cache is reached (in the example case of 1), the component that gets pruned also gets destroyed. In the documentation it states (emphasis mine):

max

The maximum number of component instances to cache. Once this number is reached, the cached component instance that was least recently accessed will be destroyed before creating a new instance.

What is actually happening?

The pruned component is never destroyed, it's just removed from the cache. In the example, the beforeDestroy hook is never called.

---

Related code:

vue/src/core/components/keep-alive.js

Lines 37 to 49 in 636c9b4

	function pruneCacheEntry (
	cache: VNodeCache,
	key: string,
	keys: Array<string>,
	current?: VNode
	) {
	const cached = cache[key]
	if (cached && (!current || cached.tag !== current.tag)) {
	cached.componentInstance.$destroy()
	}
	cache[key] = null
	remove(keys, key)
	}

[posva]

It seems not to destroy components when the max is one (in which case using keep-alive is pointless btw). I removed Vue Router from the repro to make it simpler: https://jsfiddle.net/p0te9vL3/ (max is set to 2 to show it works)

I marking this as a bug for consistency. The workaround would be not using a keep-alive if the value for max is 1

[await-ovo]

should we just validate max value just like this?

       if (parseInt(this.max) > 1) {
          var ref$1 = this;
          var cache = ref$1.cache;
          var keys = ref$1.keys;
          var key = vnode.key == null
            // same constructor may get registered as different local components
            // so cid alone is not enough (#3269)
            ? componentOptions.Ctor.cid + (componentOptions.tag ? ("::" + (componentOptions.tag)) : '')
            : vnode.key;
          if (cache[key]) {
            vnode.componentInstance = cache[key].componentInstance;
            // make current key freshest
            remove(keys, key);
            keys.push(key);
          } else {
            cache[key] = vnode;
            keys.push(key);
            // prune oldest entry
            if (this.max && keys.length > parseInt(this.max)) {
              pruneCacheEntry(cache, keys[0], keys, this._vnode);
            }
          }
  
          vnode.data.keepAlive = true;
        }

[kchung]

@posva maybe I'm confused by the prop, but when I used :max="1" I was expecting it to "keep alive" the previous component, so it'd always remember the trailing component but would destroy anything past 1

[posva]

it will cache 1 component, so as soon as you switch the cache is occupied by the new component and the previous component is removed. A cache of one entry here is the same as not having any cache

[kchung]

@posva ah that makes more sense, thanks!

[zrh122]

@posva, #9962 already fixes it, i just found out.

[posva]

@zrh122 are you sure? I haven't tested but the pr #10015 introduced a very small change that does not appear in yours. In any case, it would be nice if we could merge both for different issues, so both contributions are taken into account 🙂

[zrh122]

@zrh122 are you sure? I haven't tested but the pr #10015 introduced a very small change that does not appear in yours. In any case, it would be nice if we could merge both for different issues, so both contributions are taken into account 🙂

Yes, in my code i delay to call pruneCacheEntry until component mounted or updated, at that time this._vnode is equal to variable vnode.