Skip to content

Commit

Permalink
perf: should not trigger child update if changed prop is declared emi…
Browse files Browse the repository at this point in the history
…t listener

close #2072
  • Loading branch information
yyx990803 committed Sep 14, 2020
1 parent 4de5b11 commit 124c385
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
23 changes: 23 additions & 0 deletions packages/runtime-core/__tests__/rendererComponent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,27 @@ describe('renderer: component', () => {
render(h(Comp2), root)
expect(serializeInner(root)).toBe('<span>foo</span>')
})

// #2072
it('should not update Component if only changed props are declared emit listeners', () => {
const Comp1 = {
emits: ['foo'],
updated: jest.fn(),
render: () => null
}
const root = nodeOps.createElement('div')
render(
h(Comp1, {
onFoo: () => {}
}),
root
)
render(
h(Comp1, {
onFoo: () => {}
}),
root
)
expect(Comp1.updated).not.toHaveBeenCalled()
})
})
24 changes: 18 additions & 6 deletions packages/runtime-core/src/componentRenderUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { PatchFlags, ShapeFlags, isOn, isModelListener } from '@vue/shared'
import { warn } from './warning'
import { isHmrUpdating } from './hmr'
import { NormalizedProps } from './componentProps'
import { isEmitListener } from './componentEmits'

// mark the current rendering instance for asset resolution (e.g.
// resolveComponent, resolveDirective) during render
Expand Down Expand Up @@ -290,8 +291,9 @@ export function shouldUpdateComponent(
nextVNode: VNode,
optimized?: boolean
): boolean {
const { props: prevProps, children: prevChildren } = prevVNode
const { props: prevProps, children: prevChildren, component } = prevVNode
const { props: nextProps, children: nextChildren, patchFlag } = nextVNode
const emits = component!.emitsOptions

This comment has been minimized.

Copy link
@danyadev

danyadev Sep 16, 2020

oh
image


// Parent component's render function was hot-updated. Since this may have
// caused the child component's slots content to have changed, we need to
Expand All @@ -316,12 +318,15 @@ export function shouldUpdateComponent(
return !!nextProps
}
// presence of this flag indicates props are always non-null
return hasPropsChanged(prevProps, nextProps!)
return hasPropsChanged(prevProps, nextProps!, emits)
} else if (patchFlag & PatchFlags.PROPS) {
const dynamicProps = nextVNode.dynamicProps!
for (let i = 0; i < dynamicProps.length; i++) {
const key = dynamicProps[i]
if (nextProps![key] !== prevProps![key]) {
if (
nextProps![key] !== prevProps![key] &&
!isEmitListener(emits, key)

This comment has been minimized.

Copy link
@CyberAP

CyberAP Sep 14, 2020

Contributor

@yyx990803 should it check for vModel listeners as well?

) {
return true
}
}
Expand All @@ -343,20 +348,27 @@ export function shouldUpdateComponent(
if (!nextProps) {
return true
}
return hasPropsChanged(prevProps, nextProps)
return hasPropsChanged(prevProps, nextProps, emits)
}

return false
}

function hasPropsChanged(prevProps: Data, nextProps: Data): boolean {
function hasPropsChanged(
prevProps: Data,
nextProps: Data,
emitsOptions: ComponentInternalInstance['emitsOptions']
): boolean {
const nextKeys = Object.keys(nextProps)
if (nextKeys.length !== Object.keys(prevProps).length) {
return true
}
for (let i = 0; i < nextKeys.length; i++) {
const key = nextKeys[i]
if (nextProps[key] !== prevProps[key]) {
if (
nextProps[key] !== prevProps[key] &&
!isEmitListener(emitsOptions, key)
) {
return true
}
}
Expand Down

0 comments on commit 124c385

Please sign in to comment.