From c2fc7e334711de5a2cdf71aaaacd131f5af2e5d1 Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 3 Oct 2019 16:27:46 -0400 Subject: [PATCH] feat(compiler): force dynamicSlots flag when inside v-for or v-slot --- .../__snapshots__/vSlot.spec.ts.snap | 2 +- .../__tests__/transforms/vSlot.spec.ts | 36 ++++++++++++++++--- packages/compiler-core/src/index.ts | 4 +-- packages/compiler-core/src/transform.ts | 12 +++++++ packages/compiler-core/src/transforms/vFor.ts | 5 ++- .../compiler-core/src/transforms/vSlot.ts | 30 ++++++++++++---- 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/vSlot.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/vSlot.spec.ts.snap index 6d5193d5..8bbb14f2 100644 --- a/packages/compiler-core/__tests__/transforms/__snapshots__/vSlot.spec.ts.snap +++ b/packages/compiler-core/__tests__/transforms/__snapshots__/vSlot.spec.ts.snap @@ -159,7 +159,7 @@ return function render() { createVNode(_component_Inner, null, { default: ({ bar }) => [toString(foo), toString(bar), toString(_ctx.baz)], _compiled: true - }), + }, 256 /* DYNAMIC_SLOTS */), toString(foo), toString(_ctx.bar), toString(_ctx.baz) diff --git a/packages/compiler-core/__tests__/transforms/vSlot.spec.ts b/packages/compiler-core/__tests__/transforms/vSlot.spec.ts index 5dc22d4c..1b556424 100644 --- a/packages/compiler-core/__tests__/transforms/vSlot.spec.ts +++ b/packages/compiler-core/__tests__/transforms/vSlot.spec.ts @@ -5,7 +5,8 @@ import { generate, ElementNode, NodeTypes, - ErrorCodes + ErrorCodes, + ForNode } from '../../src' import { transformElement } from '../../src/transforms/transformElement' import { transformOn } from '../../src/transforms/vOn' @@ -17,15 +18,20 @@ import { } from '../../src/transforms/vSlot' import { CREATE_SLOTS, RENDER_LIST } from '../../src/runtimeConstants' import { createObjectMatcher } from '../testUtils' -import { PatchFlags } from '@vue/shared' +import { PatchFlags, PatchFlagNames } from '@vue/shared' +import { transformFor } from '../../src/transforms/vFor' +import { transformIf } from '../../src/transforms/vIf' function parseWithSlots(template: string, options: CompilerOptions = {}) { const ast = parse(template) transform(ast, { nodeTransforms: [ + transformIf, + transformFor, ...(options.prefixIdentifiers - ? [trackVForSlotScopes, transformExpression, trackSlotScopes] + ? [trackVForSlotScopes, transformExpression] : []), + trackSlotScopes, transformElement ], directiveTransforms: { @@ -36,7 +42,10 @@ function parseWithSlots(template: string, options: CompilerOptions = {}) { }) return { root: ast, - slots: (ast.children[0] as ElementNode).codegenNode!.arguments[2] + slots: + ast.children[0].type === NodeTypes.ELEMENT + ? ast.children[0].codegenNode!.arguments[2] + : null } } @@ -295,7 +304,12 @@ describe('compiler: transform component slots', () => { } ] } - }) + }), + // nested slot should be forced dynamic, since scope variables + // are not tracked as dependencies of the slot. + `${PatchFlags.DYNAMIC_SLOTS} /* ${ + PatchFlagNames[PatchFlags.DYNAMIC_SLOTS] + } */` ] } }, @@ -325,6 +339,18 @@ describe('compiler: transform component slots', () => { expect(generate(root, { prefixIdentifiers: true }).code).toMatchSnapshot() }) + test('should force dynamic when inside v-for', () => { + const { root } = parseWithSlots( + `
+ foo +
` + ) + 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 + '') + }) + test('named slot with v-if', () => { const { root, slots } = parseWithSlots( ` diff --git a/packages/compiler-core/src/index.ts b/packages/compiler-core/src/index.ts index eb23a9e0..f126512f 100644 --- a/packages/compiler-core/src/index.ts +++ b/packages/compiler-core/src/index.ts @@ -49,10 +49,10 @@ export function baseCompile( ? [ // order is important trackVForSlotScopes, - transformExpression, - trackSlotScopes + transformExpression ] : []), + trackSlotScopes, optimizeText, transformStyle, transformSlotOutlet, diff --git a/packages/compiler-core/src/transform.ts b/packages/compiler-core/src/transform.ts index 6d1bb90e..370d4f46 100644 --- a/packages/compiler-core/src/transform.ts +++ b/packages/compiler-core/src/transform.ts @@ -59,6 +59,12 @@ export interface TransformContext extends Required { statements: Set hoists: JSChildNode[] identifiers: { [name: string]: number | undefined } + scopes: { + vFor: number + vSlot: number + vPre: number + vOnce: number + } parent: ParentNode | null childIndex: number currentNode: RootNode | TemplateChildNode | null @@ -86,6 +92,12 @@ function createTransformContext( statements: new Set(), hoists: [], identifiers: {}, + scopes: { + vFor: 0, + vSlot: 0, + vPre: 0, + vOnce: 0 + }, prefixIdentifiers, nodeTransforms, directiveTransforms, diff --git a/packages/compiler-core/src/transforms/vFor.ts b/packages/compiler-core/src/transforms/vFor.ts index c54e9578..30fbf24d 100644 --- a/packages/compiler-core/src/transforms/vFor.ts +++ b/packages/compiler-core/src/transforms/vFor.ts @@ -46,7 +46,7 @@ export const transformFor = createStructuralDirectiveTransform( ) if (parseResult) { - const { helper, addIdentifiers, removeIdentifiers } = context + const { helper, addIdentifiers, removeIdentifiers, scopes } = context const { source, value, key, index } = parseResult // create the loop render function expression now, and add the @@ -79,6 +79,8 @@ export const transformFor = createStructuralDirectiveTransform( codegenNode }) + // bookkeeping + scopes.vFor++ if (!__BROWSER__ && context.prefixIdentifiers) { // scope management // inject identifiers to context @@ -88,6 +90,7 @@ export const transformFor = createStructuralDirectiveTransform( } return () => { + scopes.vFor-- if (!__BROWSER__ && context.prefixIdentifiers) { value && removeIdentifiers(value) key && removeIdentifiers(key) diff --git a/packages/compiler-core/src/transforms/vSlot.ts b/packages/compiler-core/src/transforms/vSlot.ts index f68d08df..7bd5cda0 100644 --- a/packages/compiler-core/src/transforms/vSlot.ts +++ b/packages/compiler-core/src/transforms/vSlot.ts @@ -32,22 +32,33 @@ const isStaticExp = (p: JSChildNode): p is SimpleExpressionNode => const defaultFallback = createSimpleExpression(`undefined`, false) -// A NodeTransform that tracks scope identifiers for scoped slots so that they -// don't get prefixed by transformExpression. This transform is only applied -// in non-browser builds with { prefixIdentifiers: true } +// A NodeTransform that: +// 1. Tracks scope identifiers for scoped slots so that they don't get prefixed +// by transformExpression. This is only applied in non-browser builds with +// { prefixIdentifiers: true }. +// 2. Track v-slot depths so that we know a slot is inside another slot. +// Note the exit callback is executed before buildSlots() on the same node, +// so only nested slots see positive numbers. export const trackSlotScopes: NodeTransform = (node, context) => { if ( node.type === NodeTypes.ELEMENT && (node.tagType === ElementTypes.COMPONENT || node.tagType === ElementTypes.TEMPLATE) ) { + // We are only checking non-empty v-slot here + // since we only care about slots that introduce scope variables. const vSlot = findDir(node, 'slot') if (vSlot) { - const { addIdentifiers, removeIdentifiers } = context const slotProps = vSlot.exp - slotProps && addIdentifiers(slotProps) + if (!__BROWSER__ && context.prefixIdentifiers) { + slotProps && context.addIdentifiers(slotProps) + } + context.scopes.vSlot++ return () => { - slotProps && removeIdentifiers(slotProps) + if (!__BROWSER__ && context.prefixIdentifiers) { + slotProps && context.removeIdentifiers(slotProps) + } + context.scopes.vSlot-- } } } @@ -94,7 +105,12 @@ export function buildSlots( const { children, loc } = node const slotsProperties: Property[] = [] const dynamicSlots: (ConditionalExpression | CallExpression)[] = [] - let hasDynamicSlots = false + + // 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 > 0 || context.scopes.vFor > 0 // 1. Check for default slot with slotProps on component itself. //