fix(engine): closes issue #225 - value and checked props are live (#471)

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");
     });
 

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.