From f219bedae8f9cab131eb5529769bcfdc91ce2912 Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Fri, 25 Jan 2019 22:34:06 -0500
Subject: [PATCH] perf: improve scoped slots change detection accuracy (#9371)

Ensure that state mutations that only affect parent scope only trigger parent update and does not affect child components with only scoped slots.
---
 flow/compiler.js                              |   1 +
 src/compiler/codegen/index.js                 |   3 +-
 src/compiler/parser/index.js                  |  18 +-
 src/core/instance/lifecycle.js                |  20 +-
 .../instance/render-helpers/resolve-slots.js  |   7 +-
 .../vdom/helpers/normalize-scoped-slots.js    |   3 +-
 .../component/component-scoped-slot.spec.js   | 341 ++++++++++--------
 test/unit/modules/compiler/codegen.spec.js    |   7 +
 8 files changed, 233 insertions(+), 167 deletions(-)

diff --git a/flow/compiler.js b/flow/compiler.js
index de1e2df9a4a..a1af7a6cec8 100644
--- a/flow/compiler.js
+++ b/flow/compiler.js
@@ -119,6 +119,7 @@ declare type ASTElement = {
   transitionMode?: string | null;
   slotName?: ?string;
   slotTarget?: ?string;
+  slotTargetDynamic?: boolean;
   slotScope?: ?string;
   scopedSlots?: { [name: string]: ASTElement };
 
diff --git a/src/compiler/codegen/index.js b/src/compiler/codegen/index.js
index bdd76be0033..9f23b37e102 100644
--- a/src/compiler/codegen/index.js
+++ b/src/compiler/codegen/index.js
@@ -354,11 +354,12 @@ function genScopedSlots (
   slots: { [key: string]: ASTElement },
   state: CodegenState
 ): string {
+  const hasDynamicKeys = Object.keys(slots).some(key => slots[key].slotTargetDynamic)
   return `scopedSlots:_u([${
     Object.keys(slots).map(key => {
       return genScopedSlot(key, slots[key], state)
     }).join(',')
-  }])`
+  }]${hasDynamicKeys ? `,true` : ``})`
 }
 
 function genScopedSlot (
diff --git a/src/compiler/parser/index.js b/src/compiler/parser/index.js
index 1f59bb93519..601dd8f26d9 100644
--- a/src/compiler/parser/index.js
+++ b/src/compiler/parser/index.js
@@ -586,6 +586,7 @@ function processSlotContent (el) {
   const slotTarget = getBindingAttr(el, 'slot')
   if (slotTarget) {
     el.slotTarget = slotTarget === '""' ? '"default"' : slotTarget
+    el.slotTargetDynamic = !!(el.attrsMap[':slot'] || el.attrsMap['v-bind:slot'])
     // preserve slot as an attribute for native shadow DOM compat
     // only for non-scoped slots.
     if (el.tag !== 'template' && !el.slotScope) {
@@ -607,8 +608,10 @@ function processSlotContent (el) {
             )
           }
         }
-        el.slotTarget = getSlotName(slotBinding)
-        el.slotScope = slotBinding.value
+        const { name, dynamic } = getSlotName(slotBinding)
+        el.slotTarget = name
+        el.slotTargetDynamic = dynamic
+        el.slotScope = slotBinding.value || `_` // force it into a scoped slot for perf
       }
     } else {
       // v-slot on component, denotes default slot
@@ -637,10 +640,11 @@ function processSlotContent (el) {
         }
         // add the component's children to its default slot
         const slots = el.scopedSlots || (el.scopedSlots = {})
-        const target = getSlotName(slotBinding)
-        const slotContainer = slots[target] = createASTElement('template', [], el)
+        const { name, dynamic } = getSlotName(slotBinding)
+        const slotContainer = slots[name] = createASTElement('template', [], el)
+        slotContainer.slotTargetDynamic = dynamic
         slotContainer.children = el.children
-        slotContainer.slotScope = slotBinding.value
+        slotContainer.slotScope = slotBinding.value || `_`
         // remove children as they are returned from scopedSlots now
         el.children = []
         // mark el non-plain so data gets generated
@@ -664,9 +668,9 @@ function getSlotName (binding) {
   }
   return dynamicKeyRE.test(name)
     // dynamic [name]
-    ? name.slice(1, -1)
+    ? { name: name.slice(1, -1), dynamic: true }
     // static name
-    : `"${name}"`
+    : { name: `"${name}"`, dynamic: false }
 }
 
 // handle <slot/> outlets
diff --git a/src/core/instance/lifecycle.js b/src/core/instance/lifecycle.js
index 3879c7b5d7f..debe94bc068 100644
--- a/src/core/instance/lifecycle.js
+++ b/src/core/instance/lifecycle.js
@@ -224,12 +224,22 @@ export function updateChildComponent (
   }
 
   // determine whether component has slot children
-  // we need to do this before overwriting $options._renderChildren
-  const hasChildren = !!(
+  // we need to do this before overwriting $options._renderChildren.
+
+  // check if there are dynamic scopedSlots (hand-written or compiled but with
+  // dynamic slot names). Static scoped slots compiled from template has the
+  // "$stable" marker.
+  const hasDynamicScopedSlot = !!(
+    (parentVnode.data.scopedSlots && !parentVnode.data.scopedSlots.$stable) ||
+    (vm.$scopedSlots !== emptyObject && !vm.$scopedSlots.$stable)
+  )
+  // Any static slot children from the parent may have changed during parent's
+  // update. Dynamic scoped slots may also have changed. In such cases, a forced
+  // update is necessary to ensure correctness.
+  const needsForceUpdate = !!(
     renderChildren ||               // has new static slots
     vm.$options._renderChildren ||  // has old static slots
-    parentVnode.data.scopedSlots || // has new scoped slots
-    vm.$scopedSlots !== emptyObject // has old scoped slots
+    hasDynamicScopedSlot
   )
 
   vm.$options._parentVnode = parentVnode
@@ -268,7 +278,7 @@ export function updateChildComponent (
   updateComponentListeners(vm, listeners, oldListeners)
 
   // resolve slots + force update if has children
-  if (hasChildren) {
+  if (needsForceUpdate) {
     vm.$slots = resolveSlots(renderChildren, parentVnode.context)
     vm.$forceUpdate()
   }
diff --git a/src/core/instance/render-helpers/resolve-slots.js b/src/core/instance/render-helpers/resolve-slots.js
index dbec386a19c..53bad202172 100644
--- a/src/core/instance/render-helpers/resolve-slots.js
+++ b/src/core/instance/render-helpers/resolve-slots.js
@@ -51,13 +51,14 @@ function isWhitespace (node: VNode): boolean {
 
 export function resolveScopedSlots (
   fns: ScopedSlotsData, // see flow/vnode
+  hasDynamicKeys?: boolean,
   res?: Object
-): { [key: string]: Function } {
-  res = res || {}
+): { [key: string]: Function, $stable: boolean } {
+  res = res || { $stable: !hasDynamicKeys }
   for (let i = 0; i < fns.length; i++) {
     const slot = fns[i]
     if (Array.isArray(slot)) {
-      resolveScopedSlots(slot, res)
+      resolveScopedSlots(slot, hasDynamicKeys, res)
     } else {
       res[slot.key] = slot.fn
     }
diff --git a/src/core/vdom/helpers/normalize-scoped-slots.js b/src/core/vdom/helpers/normalize-scoped-slots.js
index 5e5f0091e79..aa78838e770 100644
--- a/src/core/vdom/helpers/normalize-scoped-slots.js
+++ b/src/core/vdom/helpers/normalize-scoped-slots.js
@@ -14,7 +14,7 @@ export function normalizeScopedSlots (
   } else {
     res = {}
     for (const key in slots) {
-      if (slots[key]) {
+      if (slots[key] && key[0] !== '$') {
         res[key] = normalizeScopedSlot(slots[key])
       }
     }
@@ -26,6 +26,7 @@ export function normalizeScopedSlots (
     }
   }
   res._normalized = true
+  res.$stable = slots && slots.$stable
   return res
 }
 
diff --git a/test/unit/features/component/component-scoped-slot.spec.js b/test/unit/features/component/component-scoped-slot.spec.js
index 1516aaddd2f..14d0ce4a876 100644
--- a/test/unit/features/component/component-scoped-slot.spec.js
+++ b/test/unit/features/component/component-scoped-slot.spec.js
@@ -633,131 +633,69 @@ describe('Component scoped slot', () => {
   })
 
   // 2.6 new slot syntax
-  if (process.env.NEW_SLOT_SYNTAX) {
-    describe('v-slot syntax', () => {
-      const Foo = {
-        render(h) {
-          return h('div', [
-            this.$scopedSlots.default && this.$scopedSlots.default('from foo default'),
-            this.$scopedSlots.one && this.$scopedSlots.one('from foo one'),
-            this.$scopedSlots.two && this.$scopedSlots.two('from foo two')
-          ])
-        }
+  describe('v-slot syntax', () => {
+    const Foo = {
+      render(h) {
+        return h('div', [
+          this.$scopedSlots.default && this.$scopedSlots.default('from foo default'),
+          this.$scopedSlots.one && this.$scopedSlots.one('from foo one'),
+          this.$scopedSlots.two && this.$scopedSlots.two('from foo two')
+        ])
       }
+    }
 
-      const Bar = {
-        render(h) {
-          return this.$scopedSlots.default && this.$scopedSlots.default('from bar')
-        }
+    const Bar = {
+      render(h) {
+        return this.$scopedSlots.default && this.$scopedSlots.default('from bar')
       }
+    }
 
-      const Baz = {
-        render(h) {
-          return this.$scopedSlots.default && this.$scopedSlots.default('from baz')
-        }
+    const Baz = {
+      render(h) {
+        return this.$scopedSlots.default && this.$scopedSlots.default('from baz')
       }
+    }
 
-      const toNamed = (syntax, name) => syntax[0] === '#'
-        ? `#${name}` // shorthand
-        : `${syntax}:${name}` // full syntax
-
-      function runSuite(syntax) {
-        it('default slot', () => {
-          const vm = new Vue({
-            template: `<foo ${syntax}="foo">{{ foo }}<div>{{ foo }}</div></foo>`,
-            components: { Foo }
-          }).$mount()
-          expect(vm.$el.innerHTML).toBe(`from foo default<div>from foo default</div>`)
-        })
-
-        it('nested default slots', () => {
-          const vm = new Vue({
-            template: `
-              <foo ${syntax}="foo">
-                <bar ${syntax}="bar">
-                  <baz ${syntax}="baz">
-                    {{ foo }} | {{ bar }} | {{ baz }}
-                  </baz>
-                </bar>
-              </foo>
-            `,
-            components: { Foo, Bar, Baz }
-          }).$mount()
-          expect(vm.$el.innerHTML.trim()).toBe(`from foo default | from bar | from baz`)
-        })
-
-        it('named slots', () => {
-          const vm = new Vue({
-            template: `
-              <foo>
-                <template ${toNamed(syntax, 'default')}="foo">
-                  {{ foo }}
-                </template>
-                <template ${toNamed(syntax, 'one')}="one">
-                  {{ one }}
-                </template>
-                <template ${toNamed(syntax, 'two')}="two">
-                  {{ two }}
-                </template>
-              </foo>
-            `,
-            components: { Foo }
-          }).$mount()
-          expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo default from foo one from foo two`)
-        })
-
-        it('nested + named + default slots', () => {
-          const vm = new Vue({
-            template: `
-              <foo>
-                <template ${toNamed(syntax, 'one')}="one">
-                  <bar ${syntax}="bar">
-                    {{ one }} {{ bar }}
-                  </bar>
-                </template>
-                <template ${toNamed(syntax, 'two')}="two">
-                  <baz ${syntax}="baz">
-                    {{ two }} {{ baz }}
-                  </baz>
-                </template>
-              </foo>
-            `,
-            components: { Foo, Bar, Baz }
-          }).$mount()
-          expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo one from bar from foo two from baz`)
-        })
+    const toNamed = (syntax, name) => syntax[0] === '#'
+      ? `#${name}` // shorthand
+      : `${syntax}:${name}` // full syntax
 
-        it('should warn v-slot usage on non-component elements', () => {
-          const vm = new Vue({
-            template: `<div ${syntax}="foo"/>`
-          }).$mount()
-          expect(`v-slot can only be used on components or <template>`).toHaveBeenWarned()
-        })
-
-        it('should warn mixed usage', () => {
-          const vm = new Vue({
-            template: `<foo><bar slot="one" slot-scope="bar" ${syntax}="bar"></bar></foo>`,
-            components: { Foo, Bar }
-          }).$mount()
-          expect(`Unexpected mixed usage of different slot syntaxes`).toHaveBeenWarned()
-        })
-      }
+    function runSuite(syntax) {
+      it('default slot', () => {
+        const vm = new Vue({
+          template: `<foo ${syntax}="foo">{{ foo }}<div>{{ foo }}</div></foo>`,
+          components: { Foo }
+        }).$mount()
+        expect(vm.$el.innerHTML).toBe(`from foo default<div>from foo default</div>`)
+      })
 
-      // run tests for both full syntax and shorthand
-      runSuite('v-slot')
-      runSuite('#default')
+      it('nested default slots', () => {
+        const vm = new Vue({
+          template: `
+            <foo ${syntax}="foo">
+              <bar ${syntax}="bar">
+                <baz ${syntax}="baz">
+                  {{ foo }} | {{ bar }} | {{ baz }}
+                </baz>
+              </bar>
+            </foo>
+          `,
+          components: { Foo, Bar, Baz }
+        }).$mount()
+        expect(vm.$el.innerHTML.trim()).toBe(`from foo default | from bar | from baz`)
+      })
 
-      it('shorthand named slots', () => {
+      it('named slots', () => {
         const vm = new Vue({
           template: `
             <foo>
-              <template #default="foo">
+              <template ${toNamed(syntax, 'default')}="foo">
                 {{ foo }}
               </template>
-              <template #one="one">
+              <template ${toNamed(syntax, 'one')}="one">
                 {{ one }}
               </template>
-              <template #two="two">
+              <template ${toNamed(syntax, 'two')}="two">
                 {{ two }}
               </template>
             </foo>
@@ -767,62 +705,165 @@ describe('Component scoped slot', () => {
         expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo default from foo one from foo two`)
       })
 
-      it('should warn mixed root-default and named slots', () => {
+      it('nested + named + default slots', () => {
         const vm = new Vue({
           template: `
-            <foo #default="foo">
-              {{ foo }}
-              <template #one="one">
-                {{ one }}
+            <foo>
+              <template ${toNamed(syntax, 'one')}="one">
+                <bar ${syntax}="bar">
+                  {{ one }} {{ bar }}
+                </bar>
+              </template>
+              <template ${toNamed(syntax, 'two')}="two">
+                <baz ${syntax}="baz">
+                  {{ two }} {{ baz }}
+                </baz>
               </template>
             </foo>
           `,
-          components: { Foo }
+          components: { Foo, Bar, Baz }
         }).$mount()
-        expect(`default slot should also use <template>`).toHaveBeenWarned()
+        expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo one from bar from foo two from baz`)
       })
 
-      it('shorthand without scope variable', () => {
+      it('should warn v-slot usage on non-component elements', () => {
         const vm = new Vue({
-          template: `
-            <foo>
-              <template #one>one</template>
-              <template #two>two</template>
-            </foo>
-          `,
-          components: { Foo }
+          template: `<div ${syntax}="foo"/>`
         }).$mount()
-        expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`onetwo`)
+        expect(`v-slot can only be used on components or <template>`).toHaveBeenWarned()
       })
 
-      it('shorthand named slots on root', () => {
+      it('should warn mixed usage', () => {
         const vm = new Vue({
-          template: `
-            <foo #one="one">
-              {{ one }}
-            </foo>
-          `,
-          components: { Foo }
+          template: `<foo><bar slot="one" slot-scope="bar" ${syntax}="bar"></bar></foo>`,
+          components: { Foo, Bar }
         }).$mount()
-        expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo one`)
+        expect(`Unexpected mixed usage of different slot syntaxes`).toHaveBeenWarned()
       })
+    }
 
-      it('dynamic slot name', () => {
-        const vm = new Vue({
-          data: {
-            a: 'one',
-            b: 'two'
-          },
-          template: `
-            <foo>
-              <template #[a]="one">{{ one }} </template>
-              <template v-slot:[b]="two">{{ two }}</template>
-            </foo>
-          `,
-          components: { Foo }
-        }).$mount()
-        expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo one from foo two`)
-      })
+    // run tests for both full syntax and shorthand
+    runSuite('v-slot')
+    runSuite('#default')
+
+    it('shorthand named slots', () => {
+      const vm = new Vue({
+        template: `
+          <foo>
+            <template #default="foo">
+              {{ foo }}
+            </template>
+            <template #one="one">
+              {{ one }}
+            </template>
+            <template #two="two">
+              {{ two }}
+            </template>
+          </foo>
+        `,
+        components: { Foo }
+      }).$mount()
+      expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo default from foo one from foo two`)
+    })
+
+    it('should warn mixed root-default and named slots', () => {
+      const vm = new Vue({
+        template: `
+          <foo #default="foo">
+            {{ foo }}
+            <template #one="one">
+              {{ one }}
+            </template>
+          </foo>
+        `,
+        components: { Foo }
+      }).$mount()
+      expect(`default slot should also use <template>`).toHaveBeenWarned()
+    })
+
+    it('shorthand without scope variable', () => {
+      const vm = new Vue({
+        template: `
+          <foo>
+            <template #one>one</template>
+            <template #two>two</template>
+          </foo>
+        `,
+        components: { Foo }
+      }).$mount()
+      expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`onetwo`)
+    })
+
+    it('shorthand named slots on root', () => {
+      const vm = new Vue({
+        template: `
+          <foo #one="one">
+            {{ one }}
+          </foo>
+        `,
+        components: { Foo }
+      }).$mount()
+      expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`from foo one`)
     })
-  }
+
+    it('dynamic slot name', done => {
+      const vm = new Vue({
+        data: {
+          a: 'one',
+          b: 'two'
+        },
+        template: `
+          <foo>
+            <template #[a]="one">a {{ one }} </template>
+            <template v-slot:[b]="two">b {{ two }} </template>
+          </foo>
+        `,
+        components: { Foo }
+      }).$mount()
+      expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`a from foo one b from foo two`)
+      vm.a = 'two'
+      vm.b = 'one'
+      waitForUpdate(() => {
+        expect(vm.$el.innerHTML.replace(/\s+/g, ' ')).toMatch(`b from foo one a from foo two `)
+      }).then(done)
+    })
+  })
+
+  // 2.6 scoped slot perf optimization
+  it('should have accurate tracking for scoped slots', done => {
+    const parentUpdate = jasmine.createSpy()
+    const childUpdate = jasmine.createSpy()
+    const vm = new Vue({
+      template: `
+        <div>{{ parentCount }}<foo #default>{{ childCount }}</foo></div>
+      `,
+      data: {
+        parentCount: 0,
+        childCount: 0
+      },
+      updated: parentUpdate,
+      components: {
+        foo: {
+          template: `<div><slot/></div>`,
+          updated: childUpdate
+        }
+      }
+    }).$mount()
+    expect(vm.$el.innerHTML).toMatch(`0<div>0</div>`)
+
+    vm.parentCount++
+    waitForUpdate(() => {
+      expect(vm.$el.innerHTML).toMatch(`1<div>0</div>`)
+      // should only trigger parent update
+      expect(parentUpdate.calls.count()).toBe(1)
+      expect(childUpdate.calls.count()).toBe(0)
+
+      vm.childCount++
+    }).then(() => {
+      expect(vm.$el.innerHTML).toMatch(`1<div>1</div>`)
+      // should only trigger child update
+      expect(parentUpdate.calls.count()).toBe(1)
+      expect(childUpdate.calls.count()).toBe(1)
+    }).then(done)
+  })
 })
diff --git a/test/unit/modules/compiler/codegen.spec.js b/test/unit/modules/compiler/codegen.spec.js
index 7f5fbeae333..d857ecbcf02 100644
--- a/test/unit/modules/compiler/codegen.spec.js
+++ b/test/unit/modules/compiler/codegen.spec.js
@@ -229,6 +229,13 @@ describe('codegen', () => {
     )
   })
 
+  it('generate dynamic scoped slot', () => {
+    assertCodegen(
+      '<foo><template :slot="foo" slot-scope="bar">{{ bar }}</template></foo>',
+      `with(this){return _c('foo',{scopedSlots:_u([{key:foo,fn:function(bar){return [_v(_s(bar))]}}],true)})}`
+    )
+  })
+
   it('generate scoped slot with multiline v-if', () => {
     assertCodegen(
       '<foo><template v-if="\nshow\n" slot-scope="bar">{{ bar }}</template></foo>',