]> git.ipfire.org Git - thirdparty/vuejs/core.git/commitdiff
fix(compiler): cache handlers should be per-instance, fix hoist w/ cached handlers
authorEvan You <yyx990803@gmail.com>
Sun, 20 Oct 2019 21:00:11 +0000 (17:00 -0400)
committerEvan You <yyx990803@gmail.com>
Sun, 20 Oct 2019 21:00:11 +0000 (17:00 -0400)
packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap
packages/compiler-core/__tests__/codegen.spec.ts
packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap
packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts
packages/compiler-core/src/codegen.ts
packages/compiler-core/src/transform.ts
packages/compiler-core/src/transforms/hoistStatic.ts
packages/runtime-core/src/component.ts
packages/runtime-core/src/componentProxy.ts

index c4ebbc917355e2b79afe54cdcb4455e8615fa7df..851147e0fb14c2784d1ed9c5c925a9c4f975d68c 100644 (file)
@@ -12,6 +12,15 @@ return function render() {
 }"
 `;
 
+exports[`compiler: codegen CacheExpression 1`] = `
+"
+export default function render() {
+  const _ctx = this
+  const _cache = _ctx.$cache
+  return _cache[1] || (_cache[1] = foo)
+}"
+`;
+
 exports[`compiler: codegen ConditionalExpression 1`] = `
 "
 return function render() {
index 309a8440798604fdb914941909004da8560eb14e..9d7f0365f6b519a7b4e0bb9038cb62c7c8243c62 100644 (file)
@@ -137,12 +137,6 @@ describe('compiler: codegen', () => {
     expect(code).toMatchSnapshot()
   })
 
-  test('cached', () => {
-    const root = createRoot({ cached: 3 })
-    const { code } = generate(root)
-    expect(code).toMatch(`let _cached_1, _cached_2, _cached_3`)
-  })
-
   test('prefixIdentifiers: true should inject _ctx statement', () => {
     const { code } = generate(createRoot(), { prefixIdentifiers: true })
     expect(code).toMatch(`const _ctx = this\n`)
@@ -371,12 +365,19 @@ describe('compiler: codegen', () => {
   test('CacheExpression', () => {
     const { code } = generate(
       createRoot({
+        cached: 1,
         codegenNode: createCacheExpression(
           1,
           createSimpleExpression(`foo`, false)
         )
-      })
+      }),
+      {
+        mode: 'module',
+        prefixIdentifiers: true
+      }
     )
-    expect(code).toMatch(`_cached_1 || (_cached_1 = foo)`)
+    expect(code).toMatch(`const _cache = _ctx.$cache`)
+    expect(code).toMatch(`_cache[1] || (_cache[1] = foo)`)
+    expect(code).toMatchSnapshot()
   })
 })
index 8cbe9fc467fde9cb2d25e13cf350a85535b3c95e..18acfe0a3bab06b15564bb951675386da8073da6 100644 (file)
@@ -204,20 +204,18 @@ return function render() {
 `;
 
 exports[`compiler: hoistStatic transform prefixIdentifiers should NOT hoist elements with cached handlers 1`] = `
-"const _Vue = Vue
-
-let _cached_1
-
-return function render() {
-  with (this) {
-    const { createVNode: _createVNode, createBlock: _createBlock, openBlock: _openBlock } = _Vue
-    
-    return (_openBlock(), _createBlock(\\"div\\", null, [
-      _createVNode(\\"div\\", {
-        onClick: _cached_1 || (_cached_1 = $event => (_ctx.foo($event)))
+"import { createVNode, createBlock, openBlock } from \\"vue\\"
+
+export default function render() {
+  const _ctx = this
+  const _cache = _ctx.$cache
+  return (openBlock(), createBlock(\\"div\\", null, [
+    createVNode(\\"div\\", null, [
+      createVNode(\\"div\\", {
+        onClick: _cache[1] || (_cache[1] = $event => (_ctx.foo($event)))
       })
-    ]))
-  }
+    ])
+  ]))
 }"
 `;
 
index 3236276b0ae34ca793e75461c3cfae8e54ab8a1c..1fb81f93171a0319097a65418866b1b589f91a75 100644 (file)
@@ -660,14 +660,22 @@ describe('compiler: hoistStatic transform', () => {
     })
 
     test('should NOT hoist elements with cached handlers', () => {
-      const { root } = transformWithHoist(`<div><div @click="foo"/></div>`, {
-        prefixIdentifiers: true,
-        cacheHandlers: true
-      })
+      const { root } = transformWithHoist(
+        `<div><div><div @click="foo"/></div></div>`,
+        {
+          prefixIdentifiers: true,
+          cacheHandlers: true
+        }
+      )
 
       expect(root.cached).toBe(1)
       expect(root.hoists.length).toBe(0)
-      expect(generate(root).code).toMatchSnapshot()
+      expect(
+        generate(root, {
+          mode: 'module',
+          prefixIdentifiers: true
+        }).code
+      ).toMatchSnapshot()
     })
   })
 })
index 8748e0bcecf9ab429f1ac55e7c65ff504e3f3a66..946162af1155484412700ad67b8e5a8f7b7b21c9 100644 (file)
@@ -219,7 +219,6 @@ export function generate(
       }
     }
     genHoists(ast.hoists, context)
-    genCached(ast.cached, context)
     newline()
     push(`return `)
   } else {
@@ -228,7 +227,6 @@ export function generate(
       push(`import { ${ast.helpers.map(helper).join(', ')} } from "vue"\n`)
     }
     genHoists(ast.hoists, context)
-    genCached(ast.cached, context)
     newline()
     push(`export default `)
   }
@@ -253,6 +251,10 @@ export function generate(
     }
   } else {
     push(`const _ctx = this`)
+    if (ast.cached > 0) {
+      newline()
+      push(`const _cache = _ctx.$cache`)
+    }
     newline()
   }
 
@@ -318,18 +320,6 @@ function genHoists(hoists: JSChildNode[], context: CodegenContext) {
   })
 }
 
-function genCached(cached: number, context: CodegenContext) {
-  if (cached > 0) {
-    context.newline()
-    context.push(`let `)
-    for (let i = 0; i < cached; i++) {
-      context.push(`_cached_${i + 1}`)
-      if (i !== cached - 1) context.push(`, `)
-    }
-    context.newline()
-  }
-}
-
 function isText(n: string | CodegenNode) {
   return (
     isString(n) ||
@@ -632,7 +622,7 @@ function genSequenceExpression(
 }
 
 function genCacheExpression(node: CacheExpression, context: CodegenContext) {
-  context.push(`_cached_${node.index} || (_cached_${node.index} = `)
+  context.push(`_cache[${node.index}] || (_cache[${node.index}] = `)
   genNode(node.value, context)
   context.push(`)`)
 }
index 7ecf4ac41c0b1a48b88c29d51b368fa3c357442f..dd979d23583c454fd4450ca017c5d205094bfcf3 100644 (file)
@@ -219,12 +219,7 @@ function createTransformContext(
       )
     },
     cache(exp) {
-      if (cacheHandlers) {
-        context.cached++
-        return createCacheExpression(context.cached, exp)
-      } else {
-        return exp
-      }
+      return cacheHandlers ? createCacheExpression(++context.cached, exp) : exp
     }
   }
 
index 75bcfb3985be3c0046a443b4a7cbb5f5a2874dfd..9275aceb395b20ee929518098a76810e668b0ef6 100644 (file)
@@ -50,12 +50,7 @@ function walk(
       child.type === NodeTypes.ELEMENT &&
       child.tagType === ElementTypes.ELEMENT
     ) {
-      const hasBailoutProp = hasDynamicKeyOrRef(child) || hasCachedProps(child)
-      if (
-        !doNotHoistNode &&
-        !hasBailoutProp &&
-        isStaticNode(child, resultCache)
-      ) {
+      if (!doNotHoistNode && isStaticNode(child, resultCache)) {
         // whole tree is static
         child.codegenNode = context.hoist(child.codegenNode!)
         continue
@@ -67,7 +62,8 @@ function walk(
           (!flag ||
             flag === PatchFlags.NEED_PATCH ||
             flag === PatchFlags.TEXT) &&
-          !hasBailoutProp
+          !hasDynamicKeyOrRef(child) &&
+          !hasCachedProps(child)
         ) {
           const props = getNodeProps(child)
           if (props && props !== `null`) {
@@ -105,7 +101,7 @@ export function isStaticNode(
         return cached
       }
       const flag = getPatchFlag(node)
-      if (!flag) {
+      if (!flag && !hasDynamicKeyOrRef(node) && !hasCachedProps(node)) {
         // element self is static. check its children.
         for (let i = 0; i < node.children.length; i++) {
           if (!isStaticNode(node.children[i], resultCache)) {
index b3dec82fb07e023fb02c408119dbb2fdc60332d1..8dc9e758ea46e3aaa496410dafcd46bc0dfed94d 100644 (file)
@@ -83,7 +83,11 @@ export interface ComponentInternalInstance {
   render: RenderFunction | null
   effects: ReactiveEffect[] | null
   provides: Data
-  accessCache: Data
+  // cache for renderProxy access type to avoid hasOwnProperty calls
+  accessCache: Data | null
+  // cache for render function values that rely on _ctx but won't need updates
+  // after initialized (e.g. inline handlers)
+  renderCache: any[] | null
 
   components: Record<string, Component>
   directives: Record<string, Directive>
@@ -149,6 +153,7 @@ export function createComponentInstance(
     effects: null,
     provides: parent ? parent.provides : Object.create(appContext.provides),
     accessCache: null!,
+    renderCache: null,
 
     // setup context properties
     renderContext: EMPTY_OBJ,
index 87fa22ec8c17dee1fffe0ec1dfe6382b0513efca..f20caedd0b97b331553ce4441a24c201403a3477 100644 (file)
@@ -62,7 +62,7 @@ export const PublicInstanceProxyHandlers: ProxyHandler<any> = {
     // is the multiple hasOwn() calls. It's much faster to do a simple property
     // access on a plain object, so we use an accessCache object (with null
     // prototype) to memoize what access type a key corresponds to.
-    const n = accessCache[key]
+    const n = accessCache![key]
     if (n !== undefined) {
       switch (n) {
         case AccessTypes.DATA:
@@ -73,18 +73,20 @@ export const PublicInstanceProxyHandlers: ProxyHandler<any> = {
           return propsProxy![key]
       }
     } else if (data !== EMPTY_OBJ && hasOwn(data, key)) {
-      accessCache[key] = AccessTypes.DATA
+      accessCache![key] = AccessTypes.DATA
       return data[key]
     } else if (hasOwn(renderContext, key)) {
-      accessCache[key] = AccessTypes.CONTEXT
+      accessCache![key] = AccessTypes.CONTEXT
       return renderContext[key]
     } else if (hasOwn(props, key)) {
       // only cache props access if component has declared (thus stable) props
       if (type.props != null) {
-        accessCache[key] = AccessTypes.PROPS
+        accessCache![key] = AccessTypes.PROPS
       }
       // return the value from propsProxy for ref unwrapping and readonly
       return propsProxy![key]
+    } else if (key === '$cache') {
+      return target.renderCache || (target.renderCache = [])
     } else if (key === '$el') {
       return target.vnode.el
     } else if (hasOwn(publicPropertiesMap, key)) {