From d69db0b2fd8388cceb87b207292e28753395d323 Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 16 Oct 2019 15:30:21 -0400 Subject: [PATCH] feat(compiler-core/v-slot): only force dynamic slots when referencing scope vars This feature is only applied with prefixIdentifiers: true. --- .../__tests__/transforms/vModel.spec.ts | 4 +- .../__tests__/transforms/vSlot.spec.ts | 62 +++++++++++++++-- packages/compiler-core/src/index.ts | 2 +- .../compiler-core/src/transforms/vSlot.ts | 67 +++++++++++++++++-- 4 files changed, 123 insertions(+), 12 deletions(-) diff --git a/packages/compiler-core/__tests__/transforms/vModel.spec.ts b/packages/compiler-core/__tests__/transforms/vModel.spec.ts index 2efd895a..731da1e7 100644 --- a/packages/compiler-core/__tests__/transforms/vModel.spec.ts +++ b/packages/compiler-core/__tests__/transforms/vModel.spec.ts @@ -25,8 +25,8 @@ function parseWithVModel(template: string, options: CompilerOptions = {}) { nodeTransforms: [ transformFor, transformExpression, - trackSlotScopes, - transformElement + transformElement, + trackSlotScopes ], directiveTransforms: { ...options.directiveTransforms, diff --git a/packages/compiler-core/__tests__/transforms/vSlot.spec.ts b/packages/compiler-core/__tests__/transforms/vSlot.spec.ts index a430b5bf..b25897d4 100644 --- a/packages/compiler-core/__tests__/transforms/vSlot.spec.ts +++ b/packages/compiler-core/__tests__/transforms/vSlot.spec.ts @@ -7,7 +7,8 @@ import { NodeTypes, ErrorCodes, ForNode, - CallExpression + CallExpression, + ComponentNode } from '../../src' import { transformElement } from '../../src/transforms/transformElement' import { transformOn } from '../../src/transforms/vOn' @@ -32,8 +33,8 @@ function parseWithSlots(template: string, options: CompilerOptions = {}) { ...(options.prefixIdentifiers ? [trackVForSlotScopes, transformExpression] : []), - trackSlotScopes, - transformElement + transformElement, + trackSlotScopes ], directiveTransforms: { on: transformOn, @@ -347,7 +348,60 @@ describe('compiler: transform component slots', () => { const div = ((root.children[0] as ForNode).children[0] as ElementNode) .codegenNode as any const comp = div.arguments[2][0] - expect(comp.codegenNode.arguments[3]).toMatch(PatchFlags.DYNAMIC_SLOTS + '') + expect(comp.codegenNode.arguments[3]).toBe( + genFlagText(PatchFlags.DYNAMIC_SLOTS) + ) + }) + + test('should only force dynamic slots when actually using scope vars w/ prefixIdentifiers: true', () => { + function assertDynamicSlots(template: string, shouldForce: boolean) { + const { root } = parseWithSlots(template, { prefixIdentifiers: true }) + let flag: any + if (root.children[0].type === NodeTypes.FOR) { + const div = (root.children[0].children[0] as ElementNode) + .codegenNode as any + const comp = div.arguments[2][0] + flag = comp.codegenNode.arguments[3] + } else { + const innerComp = (root.children[0] as ComponentNode) + .children[0] as ComponentNode + flag = innerComp.codegenNode!.arguments[3] + } + if (shouldForce) { + expect(flag).toBe(genFlagText(PatchFlags.DYNAMIC_SLOTS)) + } else { + expect(flag).toBeUndefined() + } + } + + assertDynamicSlots( + `
+ foo +
`, + false + ) + + assertDynamicSlots( + `
+ {{ i }} +
`, + true + ) + + // reference the component's own slot variable should not force dynamic slots + assertDynamicSlots( + ` + {{ bar }} + `, + false + ) + + assertDynamicSlots( + ` + {{ foo }} + `, + true + ) }) test('named slot with v-if', () => { diff --git a/packages/compiler-core/src/index.ts b/packages/compiler-core/src/index.ts index 32c7f529..9b3fa799 100644 --- a/packages/compiler-core/src/index.ts +++ b/packages/compiler-core/src/index.ts @@ -53,9 +53,9 @@ export function baseCompile( transformExpression ] : []), - trackSlotScopes, transformSlotOutlet, transformElement, + trackSlotScopes, optimizeText, ...(options.nodeTransforms || []) // user transforms ], diff --git a/packages/compiler-core/src/transforms/vSlot.ts b/packages/compiler-core/src/transforms/vSlot.ts index 62fee3e6..d39d15f4 100644 --- a/packages/compiler-core/src/transforms/vSlot.ts +++ b/packages/compiler-core/src/transforms/vSlot.ts @@ -19,13 +19,21 @@ import { FunctionExpression, CallExpression, createCallExpression, - createArrayExpression + createArrayExpression, + IfBranchNode } from '../ast' import { TransformContext, NodeTransform } from '../transform' import { createCompilerError, ErrorCodes } from '../errors' -import { findDir, isTemplateNode, assert, isVSlot } from '../utils' +import { + findDir, + isTemplateNode, + assert, + isVSlot, + isSimpleIdentifier +} from '../utils' import { CREATE_SLOTS, RENDER_LIST } from '../runtimeHelpers' import { parseForExpression, createForLoopParams } from './vFor' +import { isObject } from '@vue/shared' const isStaticExp = (p: JSChildNode): p is SimpleExpressionNode => p.type === NodeTypes.SIMPLE_EXPRESSION && p.isStatic @@ -108,9 +116,12 @@ export function buildSlots( // If the slot is inside a v-for or another v-slot, force it to be dynamic // since it likely uses a scope variable. - // TODO: This can be further optimized to only make it dynamic when the slot - // actually uses the scope variables. - let hasDynamicSlots = context.scopes.vSlot > 1 || context.scopes.vFor > 0 + let hasDynamicSlots = context.scopes.vSlot > 0 || context.scopes.vFor > 0 + // with `prefixIdentifiers: true`, this can be further optimized to make + // it dynamic only when the slot actually uses the scope variables. + if (!__BROWSER__ && context.prefixIdentifiers) { + hasDynamicSlots = hasScopeRef(node, context.identifiers) + } // 1. Check for default slot with slotProps on component itself. // @@ -326,3 +337,49 @@ function buildDynamicSlot( createObjectProperty(`fn`, fn) ]) } + +function hasScopeRef( + node: TemplateChildNode | IfBranchNode | SimpleExpressionNode | undefined, + ids: TransformContext['identifiers'] +): boolean { + if (!node) { + return false + } + switch (node.type) { + case NodeTypes.ELEMENT: + for (let i = 0; i < node.props.length; i++) { + const p = node.props[i] + if ( + p.type === NodeTypes.DIRECTIVE && + (hasScopeRef(p.arg, ids) || hasScopeRef(p.exp, ids)) + ) { + return true + } + } + return node.children.some(c => hasScopeRef(c, ids)) + case NodeTypes.FOR: + if (hasScopeRef(node.source, ids)) { + return true + } + return node.children.some(c => hasScopeRef(c, ids)) + case NodeTypes.IF: + return node.branches.some(b => hasScopeRef(b, ids)) + case NodeTypes.IF_BRANCH: + if (hasScopeRef(node.condition, ids)) { + return true + } + return node.children.some(c => hasScopeRef(c, ids)) + case NodeTypes.SIMPLE_EXPRESSION: + return ( + !node.isStatic && + isSimpleIdentifier(node.content) && + !!ids[node.content] + ) + case NodeTypes.COMPOUND_EXPRESSION: + return node.children.some(c => isObject(c) && hasScopeRef(c, ids)) + case NodeTypes.INTERPOLATION: + return hasScopeRef(node.content, ids) + default: + return false + } +}