From 869ae19c4194bf74e486157c74ee623bc684af24 Mon Sep 17 00:00:00 2001 From: Evan You Date: Sun, 20 Oct 2019 17:00:11 -0400 Subject: [PATCH] fix(compiler): cache handlers should be per-instance, fix hoist w/ cached handlers --- .../__snapshots__/codegen.spec.ts.snap | 9 ++++++++ .../compiler-core/__tests__/codegen.spec.ts | 17 +++++++------- .../__snapshots__/hoistStatic.spec.ts.snap | 22 +++++++++---------- .../__tests__/transforms/hoistStatic.spec.ts | 18 ++++++++++----- packages/compiler-core/src/codegen.ts | 20 +++++------------ packages/compiler-core/src/transform.ts | 7 +----- .../src/transforms/hoistStatic.ts | 12 ++++------ packages/runtime-core/src/component.ts | 7 +++++- packages/runtime-core/src/componentProxy.ts | 10 +++++---- 9 files changed, 63 insertions(+), 59 deletions(-) diff --git a/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap b/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap index c4ebbc91..851147e0 100644 --- a/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap +++ b/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap @@ -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() { diff --git a/packages/compiler-core/__tests__/codegen.spec.ts b/packages/compiler-core/__tests__/codegen.spec.ts index 309a8440..9d7f0365 100644 --- a/packages/compiler-core/__tests__/codegen.spec.ts +++ b/packages/compiler-core/__tests__/codegen.spec.ts @@ -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() }) }) diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap index 8cbe9fc4..18acfe0a 100644 --- a/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap +++ b/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap @@ -204,20 +204,18 @@ return function render() { `; exports[`compiler: hoistStatic transform prefixIdentifiers should NOT hoist elements with cached handlers 1`] = ` -"const _Vue = Vue +"import { createVNode, createBlock, openBlock } from \\"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))) +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))) }) - ])) - } + ]) + ])) }" `; diff --git a/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts b/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts index 3236276b..1fb81f93 100644 --- a/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts +++ b/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts @@ -660,14 +660,22 @@ describe('compiler: hoistStatic transform', () => { }) test('should NOT hoist elements with cached handlers', () => { - const { root } = transformWithHoist(`
`, { - prefixIdentifiers: true, - cacheHandlers: true - }) + const { root } = transformWithHoist( + `
`, + { + 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() }) }) }) diff --git a/packages/compiler-core/src/codegen.ts b/packages/compiler-core/src/codegen.ts index 8748e0bc..946162af 100644 --- a/packages/compiler-core/src/codegen.ts +++ b/packages/compiler-core/src/codegen.ts @@ -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(`)`) } diff --git a/packages/compiler-core/src/transform.ts b/packages/compiler-core/src/transform.ts index 7ecf4ac4..dd979d23 100644 --- a/packages/compiler-core/src/transform.ts +++ b/packages/compiler-core/src/transform.ts @@ -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 } } diff --git a/packages/compiler-core/src/transforms/hoistStatic.ts b/packages/compiler-core/src/transforms/hoistStatic.ts index 75bcfb39..9275aceb 100644 --- a/packages/compiler-core/src/transforms/hoistStatic.ts +++ b/packages/compiler-core/src/transforms/hoistStatic.ts @@ -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)) { diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index b3dec82f..8dc9e758 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -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 directives: Record @@ -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, diff --git a/packages/runtime-core/src/componentProxy.ts b/packages/runtime-core/src/componentProxy.ts index 87fa22ec..f20caedd 100644 --- a/packages/runtime-core/src/componentProxy.ts +++ b/packages/runtime-core/src/componentProxy.ts @@ -62,7 +62,7 @@ export const PublicInstanceProxyHandlers: ProxyHandler = { // 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 = { 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)) {