Enhance patch props in element by `hasPropsChanged`
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
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.
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?