diff --git a/packages/lwc-engine/src/framework/modules/__tests__/props.spec.ts b/packages/lwc-engine/src/framework/modules/__tests__/props.spec.ts index 76ef3972e3..a8b499bbd8 100644 --- a/packages/lwc-engine/src/framework/modules/__tests__/props.spec.ts +++ b/packages/lwc-engine/src/framework/modules/__tests__/props.spec.ts @@ -3,23 +3,120 @@ import target from '../props'; describe('module/props', () => { - it('should not set the input element value when the new value matches (reconciliation)', () => { + it('should not set the input element title when the new value matches the old value from diffing (reconciliation)', () => { + const elm = {}; + let read = 0; + Object.defineProperty(elm, 'title', { + get: () => { + read += 1; + return "internal title"; + }, + set: () => { throw new Error('setter for input.title was called accidentally'); }, + configurable: false, + enumerable: true, + }); + const oldVnode = { sel: 'input', data: { props: { title: "new title" } } }; + const newVnode = { sel: 'input', data: { props: { title: "new title" } }, elm }; + + target.update(oldVnode, newVnode); + expect(read).toBe(0); + expect(elm.title).toBe("internal title"); + }); + + it('should set the input element value when the new value matches if it is not match the elm.value (especial reconciliation)', () => { const elm = {}; - let read = false; + Object.defineProperty(elm, 'value', { + value: "old value", + enumerable: true, + writable: true, + }); + const oldVnode = { sel: 'input', data: { props: { value: "new value" } } }; + const newVnode = { sel: 'input', data: { props: { value: "new value" } }, elm }; + + expect(elm.value).toBe('old value'); + // value PropertyKey is considered especial, and even if the old tracked value is equal to the new tracked value + // we still check against the element's corresponding value to be sure. + target.update(oldVnode, newVnode); + expect(elm.value).toBe("new value"); + }); + + it('should set the input element checked when the new checked matches if it is not match the elm.checked (especial reconciliation)', () => { + const elm = {}; + Object.defineProperty(elm, 'checked', { + value: "old checked", + enumerable: true, + writable: true, + }); + const oldVnode = { sel: 'input', data: { props: { checked: "new checked" } } }; + const newVnode = { sel: 'input', data: { props: { checked: "new checked" } }, elm }; + + expect(elm.checked).toBe('old checked'); + // checked PropertyKey is considered especial, and even if the old tracked value is equal to the new tracked value + // we still check against the element's corresponding value to be sure. + target.update(oldVnode, newVnode); + expect(elm.checked).toBe("new checked"); + }); + + it('should set the input element value when the new value does not match (reconciliation)', () => { + const elm = {}; + let v = "user input"; Object.defineProperty(elm, 'value', { get: () => { - read = true; - return "new value"; + return v; + }, + set: (value) => { + v = value; }, - set: () => { throw new Error('setter for input.value was called accidentally'); }, configurable: false, enumerable: true, }); - const oldVnode = { data: { props: { value: "old value" } } }; - const newVnode = { data: { props: { value: "new value" } }, elm }; + const oldVnode = { sel: 'input', data: { props: { value: "old value" } } }; + const newVnode = { sel: 'input', data: { props: { value: "new value" } }, elm }; target.update(oldVnode, newVnode); - expect(read).toBe(true); + expect(v).toBe("new value"); + expect(elm.value).toBe("new value"); + }); + + it('should set the textarea element value when the new value does not match (reconciliation)', () => { + const elm = {}; + let v = "user input"; + Object.defineProperty(elm, 'value', { + get: () => { + return v; + }, + set: (value) => { + v = value; + }, + configurable: false, + enumerable: true, + }); + const oldVnode = { sel: 'textarea', data: { props: { value: "old value" } } }; + const newVnode = { sel: 'textarea', data: { props: { value: "new value" } }, elm }; + + target.update(oldVnode, newVnode); + expect(v).toBe("new value"); + expect(elm.value).toBe("new value"); + }); + + it('should set the select element value when the new value does not match (reconciliation)', () => { + const elm = {}; + let v = "user input"; + Object.defineProperty(elm, 'value', { + get: () => { + return v; + }, + set: (value) => { + v = value; + }, + configurable: false, + enumerable: true, + }); + const oldVnode = { sel: 'select', data: { props: { value: "old value" } } }; + const newVnode = { sel: 'select', data: { props: { value: "new value" } }, elm }; + + target.update(oldVnode, newVnode); + expect(v).toBe("new value"); expect(elm.value).toBe("new value"); }); diff --git a/packages/lwc-engine/src/framework/modules/props.ts b/packages/lwc-engine/src/framework/modules/props.ts index b8958cd243..fccc727f63 100644 --- a/packages/lwc-engine/src/framework/modules/props.ts +++ b/packages/lwc-engine/src/framework/modules/props.ts @@ -1,16 +1,28 @@ import assert from "../assert"; -import { isUndefined, keys, StringToLowerCase } from "../language"; -import { ViewModelReflection, EmptyObject, getInternalField } from "../utils"; +import { isUndefined, keys, StringToLowerCase, create } from "../language"; +import { ViewModelReflection, getInternalField } from "../utils"; import { prepareForPropUpdate } from "../decorators/api"; import { VNode, Module } from "../../3rdparty/snabbdom/types"; import { getAttrNameFromPropName } from "../attributes"; +const EspecialTagAndPropMap = create(null, { + input: { value: create(null, { value: { value: 1 }, checked: { value: 1 } }) }, + select: { value: create(null, { value: { value: 1 } }) }, + textarea: { value: create(null, { value: { value: 1 } }) }, +}); + +function isLiveBindingProp(sel: string, key: string): boolean { + // checked and value properties are considered especial, and even if the old tracked value is equal to the new tracked value + // we still check against the element's corresponding value to be sure. + return sel in EspecialTagAndPropMap && key in EspecialTagAndPropMap[sel]; +} + function update(oldVnode: VNode, vnode: VNode) { const props = vnode.data.props; if (isUndefined(props)) { return; } - let oldProps = oldVnode.data.props; + const oldProps = oldVnode.data.props; if (oldProps === props) { return; } @@ -21,29 +33,25 @@ function update(oldVnode: VNode, vnode: VNode) { let key: string; let cur: any; - let old: any; const elm = vnode.elm as Element; const vm = getInternalField(elm, ViewModelReflection); - oldProps = isUndefined(oldProps) ? EmptyObject : oldProps; + const isFirstPatch = isUndefined(oldProps); + const isCustomElement = !isUndefined(vm); + const { sel } = vnode; for (key in props) { cur = props[key]; - old = (oldProps as any)[key]; if (process.env.NODE_ENV !== 'production') { - if (old !== cur && !(key in elm)) { + if (!(key in elm)) { // TODO: this should never really happen because the compiler should always validate assert.fail(`Unknown public property "${key}" of element <${StringToLowerCase.call(elm.tagName)}>. This is likely a typo on the corresponding attribute "${getAttrNameFromPropName(key)}".`); } } - if (old !== cur && (key in elm) && (key !== 'value' || elm[key] !== cur)) { - if (process.env.NODE_ENV !== 'production') { - if (elm[key] === cur && old !== undefined) { - console.warn(`Unnecessary update of property "${key}" in element <${StringToLowerCase.call(elm.tagName)}>.`); // tslint:disable-line - } - } - if (!isUndefined(vm)) { + // if it is the first time this element is patched, or the current value is different to the previous value... + if (isFirstPatch || cur !== (isLiveBindingProp(sel as string, key) ? elm[key] : (oldProps as any)[key])) { + if (isCustomElement) { prepareForPropUpdate(vm); // this is just in case the vnode is actually a custom element } // touching the dom if the prop really changes.