Subscribe on changes!

Enhance patch props in element by `hasPropsChanged`

avatar
Apr 21st 2022

What problem does this feature solve?

Consider a test case like this:

it('should not update element props which is already mounted when props are same', () => {
  render(h('div', { id: 'bar' }, ['foo']), root)
  expect(inner(root)).toBe('<div id="bar">foo</div>')

  render(h('div', { id: 'bar' }, ['foo']), root)
  expect(inner(root)).toBe('<div id="bar">foo</div>')
})

oldProps and newProps are actually the same when updating elemtent props, but since they are objects, oldProps !== newProps still return true like this line in renderer

// patchProps
if (oldProps !== newProps) {
    for (const key in newProps) {/* handle newProps */}
    if (oldProps !== EMPTY_OBJ) {/* handle oldProps */}
    if ('value' in newProps) {/* handle 'value' prop */}
}

Is it possible to consider using hasPropsChanged in componentRenderUtils instead of oldProps !== newProps to improve performance. Please confirm if the scheme is feasible, I tried modifying to hasPropsChanged and all tests passed, ready to PR.

What does the proposed API look like?

no proposed API

avatar
Apr 21st 2022

In the body of for (const key in newProps) {/* handle newProps */}, which you commented out, we do check wether the old value is equal to the new one before doing an actual patch, so we kinds already do what hasPropsChanged does. though yes, we do a second loop over oldProps after this first one.

Your proposed change would effectively save one loop over the props keys when props did not change, and add one more loop over the props keys when props did change.

This seems like a small net benefit in (the likely common) scenarios where you have more elements with unchanged props than elements with changed props in a given re-render.

avatar
Apr 21st 2022

Your proposed change would effectively save one loop over the props keys when props did not change, and add one more loop over the props keys when props did change.

This seems like a small net benefit in (the likely common) scenarios where you have more elements with unchanged props than elements with changed props in a given re-render.

I agree with you, it seems to be a balance issue at the design level, one scenario will save one loop, and one scenario will add one loop. From the design perspective of the vue.js framework, do I need to modify this part of the code?

avatar
May 13th 2022

see #5857