fix(compiler-core): prevent cached array children from retaining detached dom nodes

Author: edison1105

packages/compiler-core/__tests__/transforms/cacheStatic.spec.ts

@@ -170,11 +170,6 @@ describe('compiler: cacheStatic transform', () => {
         {
           /* _ slot flag */
         },
-        {
-          type: NodeTypes.JS_PROPERTY,
-          key: { content: '__' },
-          value: { content: '[0]' },
-        },
       ],
     })
   })
@@ -202,11 +197,6 @@ describe('compiler: cacheStatic transform', () => {
         {
           /* _ slot flag */
         },
-        {
-          type: NodeTypes.JS_PROPERTY,
-          key: { content: '__' },
-          value: { content: '[0]' },
-        },
       ],
     })
   })

packages/compiler-core/src/ast.ts

@@ -420,6 +420,7 @@ export interface CacheExpression extends Node {
   needPauseTracking: boolean
   inVOnce: boolean
   needArraySpread: boolean
+  cachedAsArray: boolean
 }
 
 export interface MemoExpression extends CallExpression {
@@ -784,6 +785,7 @@ export function createCacheExpression(
     needPauseTracking: needPauseTracking,
     inVOnce,
     needArraySpread: false,
+    cachedAsArray: false,
     loc: locStub,
   }
 }

packages/compiler-core/src/codegen.ts

@@ -1012,7 +1012,7 @@ function genConditionalExpression(
 
 function genCacheExpression(node: CacheExpression, context: CodegenContext) {
   const { push, helper, indent, deindent, newline } = context
-  const { needPauseTracking, needArraySpread } = node
+  const { needPauseTracking, needArraySpread, cachedAsArray } = node
   if (needArraySpread) {
     push(`[...(`)
   }
@@ -1027,6 +1027,9 @@ function genCacheExpression(node: CacheExpression, context: CodegenContext) {
   }
   push(`_cache[${node.index}] = `)
   genNode(node.value, context)
+  if (cachedAsArray) {
+    push(`, _cache[${node.index}].patchFlag = -1, _cache[${node.index}]`)
+  }
   if (needPauseTracking) {
     push(`).cacheIndex = ${node.index},`)
     newline()

packages/compiler-core/src/transforms/cacheStatic.ts

@@ -12,14 +12,11 @@ import {
   type RootNode,
   type SimpleExpressionNode,
   type SlotFunctionExpression,
-  type SlotsObjectProperty,
   type TemplateChildNode,
   type TemplateNode,
   type TextCallNode,
   type VNodeCall,
   createArrayExpression,
-  createObjectProperty,
-  createSimpleExpression,
   getVNodeBlockHelper,
   getVNodeHelper,
 } from '../ast'
@@ -70,7 +67,10 @@ function walk(
   inFor = false,
 ) {
   const { children } = node
-  const toCache: (PlainElementNode | TextCallNode)[] = []
+  const toCacheMap = new Map<
+    PlainElementNode | TextCallNode,
+    undefined | (() => void)
+  >()
   for (let i = 0; i < children.length; i++) {
     const child = children[i]
     // only plain elements & text calls are eligible for caching.
@@ -83,8 +83,11 @@ function walk(
         : getConstantType(child, context)
       if (constantType > ConstantTypes.NOT_CONSTANT) {
         if (constantType >= ConstantTypes.CAN_CACHE) {
-          ;(child.codegenNode as VNodeCall).patchFlag = PatchFlags.CACHED
-          toCache.push(child)
+          toCacheMap.set(
+            child,
+            () =>
+              ((child.codegenNode as VNodeCall).patchFlag = PatchFlags.CACHED),
+          )
           continue
         }
       } else {
@@ -115,16 +118,17 @@ function walk(
         ? ConstantTypes.NOT_CONSTANT
         : getConstantType(child, context)
       if (constantType >= ConstantTypes.CAN_CACHE) {
-        if (
-          child.codegenNode.type === NodeTypes.JS_CALL_EXPRESSION &&
-          child.codegenNode.arguments.length > 0
-        ) {
-          child.codegenNode.arguments.push(
-            PatchFlags.CACHED +
-              (__DEV__ ? ` /* ${PatchFlagNames[PatchFlags.CACHED]} */` : ``),
-          )
-        }
-        toCache.push(child)
+        toCacheMap.set(child, () => {
+          if (
+            child.codegenNode.type === NodeTypes.JS_CALL_EXPRESSION &&
+            child.codegenNode.arguments.length > 0
+          ) {
+            child.codegenNode.arguments.push(
+              PatchFlags.CACHED +
+                (__DEV__ ? ` /* ${PatchFlagNames[PatchFlags.CACHED]} */` : ``),
+            )
+          }
+        })
         continue
       }
     }
@@ -157,8 +161,7 @@ function walk(
   }
 
   let cachedAsArray = false
-  const slotCacheKeys = []
-  if (toCache.length === children.length && node.type === NodeTypes.ELEMENT) {
+  if (toCacheMap.size === children.length && node.type === NodeTypes.ELEMENT) {
     if (
       node.tagType === ElementTypes.ELEMENT &&
       node.codegenNode &&
@@ -181,7 +184,6 @@ function walk(
       // default slot
       const slot = getSlotNode(node.codegenNode, 'default')
       if (slot) {
-        slotCacheKeys.push(context.cached.length)
         slot.returns = getCacheExpression(
           createArrayExpression(slot.returns as TemplateChildNode[]),
         )
@@ -205,7 +207,6 @@ function walk(
         slotName.arg &&
         getSlotNode(parent.codegenNode, slotName.arg)
       if (slot) {
-        slotCacheKeys.push(context.cached.length)
         slot.returns = getCacheExpression(
           createArrayExpression(slot.returns as TemplateChildNode[]),
         )
@@ -215,40 +216,24 @@ function walk(
   }
 
   if (!cachedAsArray) {
-    for (const child of toCache) {
-      slotCacheKeys.push(context.cached.length)
+    for (const [child, setupCache] of toCacheMap) {
+      if (setupCache) setupCache()
       child.codegenNode = context.cache(child.codegenNode!)
     }
   }
 
-  // put the slot cached keys on the slot object, so that the cache
-  // can be removed when component unmounting to prevent memory leaks
-  if (
-    slotCacheKeys.length &&
-    node.type === NodeTypes.ELEMENT &&
-    node.tagType === ElementTypes.COMPONENT &&
-    node.codegenNode &&
-    node.codegenNode.type === NodeTypes.VNODE_CALL &&
-    node.codegenNode.children &&
-    !isArray(node.codegenNode.children) &&
-    node.codegenNode.children.type === NodeTypes.JS_OBJECT_EXPRESSION
-  ) {
-    node.codegenNode.children.properties.push(
-      createObjectProperty(
-        `__`,
-        createSimpleExpression(JSON.stringify(slotCacheKeys), false),
-      ) as SlotsObjectProperty,
-    )
-  }
-
-  function getCacheExpression(value: JSChildNode): CacheExpression {
+  function getCacheExpression(
+    value: JSChildNode,
+    cachedAsArray: boolean = true,
+  ): CacheExpression {
     const exp = context.cache(value)
     // #6978, #7138, #7114
     // a cached children array inside v-for can caused HMR errors since
     // it might be mutated when mounting the first item
     if (inFor && context.hmr) {
       exp.needArraySpread = true
     }
+    exp.cachedAsArray = cachedAsArray
     return exp
   }
 
@@ -268,7 +253,7 @@ function walk(
     }
   }
 
-  if (toCache.length && context.transformHoist) {
+  if (toCacheMap.size && context.transformHoist) {
     context.transformHoist(children, context, node)
   }
 }

packages/runtime-core/__tests__/componentSlots.spec.ts

@@ -56,14 +56,10 @@ describe('component: slots', () => {
         expect(Object.getOwnPropertyDescriptor(slots, '_')!.enumerable).toBe(
           false,
         )
-        expect(slots).toHaveProperty('__')
-        expect(Object.getOwnPropertyDescriptor(slots, '__')!.enumerable).toBe(
-          false,
-        )
         return h('div')
       },
     }
-    const slots = { foo: () => {}, _: 1, __: [1] }
+    const slots = { foo: () => {}, _: 1 }
     render(createBlock(Comp, null, slots), nodeOps.createElement('div'))
   })
 

packages/runtime-core/src/componentSlots.ts

@@ -79,15 +79,10 @@ export type RawSlots = {
    * @internal
    */
   _?: SlotFlags
-  /**
-   * cache indexes for slot content
-   * @internal
-   */
-  __?: number[]
 }
 
 const isInternalKey = (key: string) =>
-  key === '_' || key === '__' || key === '_ctx' || key === '$stable'
+  key === '_' || key === '_ctx' || key === '$stable'
 
 const normalizeSlotValue = (value: unknown): VNode[] =>
   isArray(value)
@@ -194,10 +189,6 @@ export const initSlots = (
 ): void => {
   const slots = (instance.slots = createInternalObject())
   if (instance.vnode.shapeFlag & ShapeFlags.SLOTS_CHILDREN) {
-    const cacheIndexes = (children as RawSlots).__
-    // make cache indexes marker non-enumerable
-    if (cacheIndexes) def(slots, '__', cacheIndexes, true)
-
     const type = (children as RawSlots)._
     if (type) {
       assignSlots(slots, children as Slots, optimized)

packages/runtime-core/src/helpers/renderSlot.ts

@@ -9,6 +9,7 @@ import {
   Fragment,
   type VNode,
   type VNodeArrayChildren,
+  cloneVNode,
   createBlock,
   createVNode,
   isVNode,
@@ -71,12 +72,24 @@ export function renderSlot(
     ;(slot as ContextualRenderFn)._d = false
   }
   openBlock()
-  const validSlotContent = slot && ensureValidVNode(slot(props))
+  let validSlotContent = slot && ensureValidVNode(slot(props))
   const slotKey =
     props.key ||
     // slot content array of a dynamic conditional slot may have a branch
     // key attached in the `createSlots` helper, respect that
     (validSlotContent && (validSlotContent as any).key)
+
+  // if slot content is an cached array, deep clone it to prevent
+  // cached vnodes from retaining detached DOM nodes
+  if (
+    validSlotContent &&
+    (validSlotContent as any).patchFlag === PatchFlags.CACHED
+  ) {
+    validSlotContent = (validSlotContent as VNode[]).map(child =>
+      cloneVNode(child),
+    )
+  }
+
   const rendered = createBlock(
     Fragment,
     {

packages/runtime-core/src/renderer.ts

@@ -2277,17 +2277,7 @@ function baseCreateRenderer(
       unregisterHMR(instance)
     }
 
-    const {
-      bum,
-      scope,
-      job,
-      subTree,
-      um,
-      m,
-      a,
-      parent,
-      slots: { __: slotCacheKeys },
-    } = instance
+    const { bum, scope, job, subTree, um, m, a } = instance
     invalidateMount(m)
     invalidateMount(a)
 
@@ -2296,13 +2286,6 @@ function baseCreateRenderer(
       invokeArrayFns(bum)
     }
 
-    // remove slots content from parent renderCache
-    if (parent && isArray(slotCacheKeys)) {
-      slotCacheKeys.forEach(v => {
-        parent.renderCache[v] = undefined
-      })
-    }
-
     if (
       __COMPAT__ &&
       isCompatEnabled(DeprecationTypes.INSTANCE_EVENT_HOOKS, instance)

packages/runtime-core/src/vnode.ts

@@ -471,7 +471,11 @@ function createBaseVNode(
     ref: props && normalizeRef(props),
     scopeId: currentScopeId,
     slotScopeIds: null,
-    children,
+    children:
+      // children is a cached array
+      isArray(children) && (children as any).patchFlag === PatchFlags.CACHED
+        ? (children as VNode[]).map(child => cloneVNode(child))
+        : children,
     component: null,
     suspense: null,
     ssContent: null,
@@ -680,7 +684,9 @@ export function cloneVNode<T, U>(
     scopeId: vnode.scopeId,
     slotScopeIds: vnode.slotScopeIds,
     children:
-      __DEV__ && patchFlag === PatchFlags.CACHED && isArray(children)
+      // if vnode is cached, deep clone it's children to prevent cached children
+      // from retaining detached DOM nodes
+      patchFlag === PatchFlags.CACHED && isArray(children)
         ? (children as VNode[]).map(deepCloneVNode)
         : children,
     target: vnode.target,
@@ -738,7 +744,7 @@ export function cloneVNode<T, U>(
 }
 
 /**
- * Dev only, for HMR of hoisted vnodes reused in v-for
+ * for HMR of hoisted vnodes reused in v-for
  * https://github.com/vitejs/vite/issues/2022
  */
 function deepCloneVNode(vnode: VNode): VNode {