]> git.ipfire.org Git - thirdparty/vuejs/core.git/commitdiff
feat(compiler-core/v-slot): only force dynamic slots when referencing scope vars
authorEvan You <yyx990803@gmail.com>
Wed, 16 Oct 2019 19:30:21 +0000 (15:30 -0400)
committerEvan You <yyx990803@gmail.com>
Wed, 16 Oct 2019 19:35:04 +0000 (15:35 -0400)
This feature is only applied with prefixIdentifiers: true.

packages/compiler-core/__tests__/transforms/vModel.spec.ts
packages/compiler-core/__tests__/transforms/vSlot.spec.ts
packages/compiler-core/src/index.ts
packages/compiler-core/src/transforms/vSlot.ts

index 2efd895a6f11695f20f70eeb954b803d90df4b0f..731da1e7dcda6a6e4f8b572f75da352eb58420fd 100644 (file)
@@ -25,8 +25,8 @@ function parseWithVModel(template: string, options: CompilerOptions = {}) {
     nodeTransforms: [
       transformFor,
       transformExpression,
-      trackSlotScopes,
-      transformElement
+      transformElement,
+      trackSlotScopes
     ],
     directiveTransforms: {
       ...options.directiveTransforms,
index a430b5bfd62961ef836ccd5e210e01b5ab579ba2..b25897d4dc9dfe3313f1b1b8dc237975d6a1953b 100644 (file)
@@ -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(
+      `<div v-for="i in list">
+        <Comp v-slot="bar">foo</Comp>
+      </div>`,
+      false
+    )
+
+    assertDynamicSlots(
+      `<div v-for="i in list">
+        <Comp v-slot="bar">{{ i }}</Comp>
+      </div>`,
+      true
+    )
+
+    // reference the component's own slot variable should not force dynamic slots
+    assertDynamicSlots(
+      `<Comp v-slot="foo">
+        <Comp v-slot="bar">{{ bar }}</Comp>
+      </Comp>`,
+      false
+    )
+
+    assertDynamicSlots(
+      `<Comp v-slot="foo">
+        <Comp v-slot="bar">{{ foo }}</Comp>
+      </Comp>`,
+      true
+    )
   })
 
   test('named slot with v-if', () => {
index 32c7f529ed970bf5143ca4e1ee072c7a0075f6e9..9b3fa799ba86a2c5cddf4e86580c837de45e9e9c 100644 (file)
@@ -53,9 +53,9 @@ export function baseCompile(
             transformExpression
           ]
         : []),
-      trackSlotScopes,
       transformSlotOutlet,
       transformElement,
+      trackSlotScopes,
       optimizeText,
       ...(options.nodeTransforms || []) // user transforms
     ],
index 62fee3e6a7dfb5d654e5b0875f52a03c8ca49728..d39d15f45604a794ae3ce9de82e755616ffc72d1 100644 (file)
@@ -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.
   //    <Comp v-slot="{ prop }"/>
@@ -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
+  }
+}