[Live] Fixed bug with "unsynced" inputs and <form data-model>

src/LiveComponent/assets/dist/live_controller.js

@@ -1184,7 +1184,7 @@ function normalizeAttributesForComparison(element) {
     });
 }
 
-function executeMorphdom(rootFromElement, rootToElement, modifiedElements, getElementValue, childComponents, findChildComponent, getKeyFromElement) {
+function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements, getElementValue, childComponents, findChildComponent, getKeyFromElement) {
     const childComponentMap = new Map();
     childComponents.forEach((childComponent) => {
         childComponentMap.set(childComponent.element, childComponent);
@@ -1215,7 +1215,7 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedElements, getEl
             if (childComponent) {
                 return childComponent.updateFromNewElement(toEl);
             }
-            if (modifiedElements.includes(fromEl)) {
+            if (modifiedFieldElements.includes(fromEl)) {
                 setValueOnElement(toEl, getElementValue(fromEl));
             }
             if (fromEl.isEqualNode(toEl)) {
@@ -1238,35 +1238,6 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedElements, getEl
     });
 }
 
-/******************************************************************************
-Copyright (c) Microsoft Corporation.
-
-Permission to use, copy, modify, and/or distribute this software for any
-purpose with or without fee is hereby granted.
-
-THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
-REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
-INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
-OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-PERFORMANCE OF THIS SOFTWARE.
-***************************************************************************** */
-
-function __classPrivateFieldGet(receiver, state, kind, f) {
-    if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a getter");
-    if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it");
-    return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver);
-}
-
-function __classPrivateFieldSet(receiver, state, value, kind, f) {
-    if (kind === "m") throw new TypeError("Private method is not writable");
-    if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a setter");
-    if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot write private member to an object whose class did not declare it");
-    return (kind === "a" ? f.call(receiver, value) : f ? f.value = value : state.set(receiver, value)), value;
-}
-
-var _UnsyncedInputContainer_mappedFields, _UnsyncedInputContainer_unmappedFields;
 class UnsyncedInputsTracker {
     constructor(component, modelElementResolver) {
         this.elementEventListeners = [
@@ -1307,36 +1278,51 @@ class UnsyncedInputsTracker {
         this.unsyncedInputs.add(element, modelName);
     }
     getUnsyncedInputs() {
-        return this.unsyncedInputs.all();
+        return this.unsyncedInputs.allUnsyncedInputs();
+    }
+    getUnsyncedModels() {
+        return Array.from(this.unsyncedInputs.getUnsyncedModelNames());
     }
-    getModifiedModels() {
-        return Array.from(this.unsyncedInputs.getModifiedModels());
+    resetUnsyncedFields() {
+        this.unsyncedInputs.resetUnsyncedFields();
     }
 }
 class UnsyncedInputContainer {
     constructor() {
-        _UnsyncedInputContainer_mappedFields.set(this, void 0);
-        _UnsyncedInputContainer_unmappedFields.set(this, []);
-        __classPrivateFieldSet(this, _UnsyncedInputContainer_mappedFields, new Map(), "f");
+        this.unsyncedNonModelFields = [];
+        this.unsyncedModelNames = [];
+        this.unsyncedModelFields = new Map();
     }
     add(element, modelName = null) {
         if (modelName) {
-            __classPrivateFieldGet(this, _UnsyncedInputContainer_mappedFields, "f").set(modelName, element);
+            this.unsyncedModelFields.set(modelName, element);
+            if (!this.unsyncedModelNames.includes(modelName)) {
+                this.unsyncedModelNames.push(modelName);
+            }
             return;
         }
-        __classPrivateFieldGet(this, _UnsyncedInputContainer_unmappedFields, "f").push(element);
+        this.unsyncedNonModelFields.push(element);
     }
-    all() {
-        return [...__classPrivateFieldGet(this, _UnsyncedInputContainer_unmappedFields, "f"), ...__classPrivateFieldGet(this, _UnsyncedInputContainer_mappedFields, "f").values()];
+    resetUnsyncedFields() {
+        this.unsyncedModelFields.forEach((value, key) => {
+            if (!this.unsyncedModelNames.includes(key)) {
+                this.unsyncedModelFields.delete(key);
+            }
+        });
+    }
+    allUnsyncedInputs() {
+        return [...this.unsyncedNonModelFields, ...this.unsyncedModelFields.values()];
     }
     markModelAsSynced(modelName) {
-        __classPrivateFieldGet(this, _UnsyncedInputContainer_mappedFields, "f").delete(modelName);
+        const index = this.unsyncedModelNames.indexOf(modelName);
+        if (index !== -1) {
+            this.unsyncedModelNames.splice(index, 1);
+        }
     }
-    getModifiedModels() {
-        return Array.from(__classPrivateFieldGet(this, _UnsyncedInputContainer_mappedFields, "f").keys());
+    getUnsyncedModelNames() {
+        return this.unsyncedModelNames;
     }
 }
-_UnsyncedInputContainer_mappedFields = new WeakMap(), _UnsyncedInputContainer_unmappedFields = new WeakMap();
 
 class HookManager {
     constructor() {
@@ -1452,7 +1438,7 @@ class Component {
         return promise;
     }
     getUnsyncedModels() {
-        return this.unsyncedInputsTracker.getModifiedModels();
+        return this.unsyncedInputsTracker.getUnsyncedModels();
     }
     addChild(child, modelBindings = []) {
         if (!child.id) {
@@ -1521,6 +1507,7 @@ class Component {
     performRequest() {
         const thisPromiseResolve = this.nextRequestPromiseResolve;
         this.resetPromise();
+        this.unsyncedInputsTracker.resetUnsyncedFields();
         this.backendRequest = this.backend.makeRequest(this.valueStore.all(), this.pendingActions, this.valueStore.updatedModels, this.getChildrenFingerprints());
         this.hooks.triggerHook('loading.state:started', this.element, this.backendRequest);
         this.pendingActions = [];

src/LiveComponent/assets/src/Component/UnsyncedInputsTracker.ts

@@ -58,49 +58,87 @@ export default class {
     }
 
     getUnsyncedInputs(): HTMLElement[] {
-        return this.unsyncedInputs.all();
+        return this.unsyncedInputs.allUnsyncedInputs();
     }
 
-    getModifiedModels(): string[] {
-        return Array.from(this.unsyncedInputs.getModifiedModels());
+    getUnsyncedModels(): string[] {
+        return Array.from(this.unsyncedInputs.getUnsyncedModelNames());
+    }
+
+    resetUnsyncedFields(): void {
+        this.unsyncedInputs.resetUnsyncedFields();
     }
 }
 
 /**
  * Tracks field & models whose values are "unsynced".
  *
- * Unsynced means that the value has been updated inside of
+ * For a model, unsynced means that the value has been updated inside of
  * a field (e.g. an input), but that this new value hasn't
  * yet been set onto the actual model data. It is "unsynced"
  * from the underlying model data.
+ *
+ * For a field, unsynced means that it is "modified on the client side". In
+ * other words, the field's value in the browser would be different than the
+ * one returned from the server. This can happen because a field has no model
+ * (and so it is permanently unsynced once changed) or the field has been changed
+ * and the corresponding model has not yet been sent to the server.
+ *
+ * Note: a "model" can become synced when that value is set back
+ * onto the data store. But the corresponding field will
+ * remain unsynced until the next Ajax call starts.
  */
 export class UnsyncedInputContainer {
-    #mappedFields: Map<string, HTMLElement>;
-    #unmappedFields: Array<HTMLElement> = [];
+    private unsyncedModelFields: Map<string, HTMLElement>;
+    private unsyncedNonModelFields: Array<HTMLElement> = [];
+    private unsyncedModelNames: Array<string> = [];
 
     constructor() {
-        this.#mappedFields = new Map();
+        this.unsyncedModelFields = new Map();
     }
 
     add(element: HTMLElement, modelName: string|null = null) {
         if (modelName) {
-            this.#mappedFields.set(modelName, element);
+            this.unsyncedModelFields.set(modelName, element);
+            if (!this.unsyncedModelNames.includes(modelName)) {
+                this.unsyncedModelNames.push(modelName);
+            }
 
             return;
         }
 
-        this.#unmappedFields.push(element);
+        this.unsyncedNonModelFields.push(element);
     }
 
-    all(): HTMLElement[] {
-        return [...this.#unmappedFields, ...this.#mappedFields.values()]
+    /**
+     * Mark all fields as synced, except for those not bound to a model or whose
+     * values are still dirty.
+     */
+    resetUnsyncedFields(): void {
+        // clear out all unsynced fields, except those where the value is still unsynced
+        this.unsyncedModelFields.forEach((value, key) => {
+            if (!this.unsyncedModelNames.includes(key)) {
+                this.unsyncedModelFields.delete(key);
+            }
+        });
+    }
+
+    allUnsyncedInputs(): HTMLElement[] {
+        return [...this.unsyncedNonModelFields, ...this.unsyncedModelFields.values()]
     }
 
     markModelAsSynced(modelName: string): void {
-        this.#mappedFields.delete(modelName);
+        const index = this.unsyncedModelNames.indexOf(modelName);
+        if (index !== -1) {
+            this.unsyncedModelNames.splice(index, 1);
+        }
     }
 
-    getModifiedModels(): string[] {
-        return Array.from(this.#mappedFields.keys());
+    /**
+     * Returns a list of models whose fields have been modified, but whose values
+     * have not yet been set onto the data store.
+     */
+    getUnsyncedModelNames(): string[] {
+        return this.unsyncedModelNames;
     }
 }

src/LiveComponent/assets/src/Component/index.ts

@@ -164,8 +164,12 @@ export default class Component {
         return promise;
     }
 
+    /**
+     * Returns an array of models the user has modified, but whose model has not
+     * yet been updated.
+     */
     getUnsyncedModels(): string[] {
-        return this.unsyncedInputsTracker.getModifiedModels();
+        return this.unsyncedInputsTracker.getUnsyncedModels();
     }
 
     addChild(child: Component, modelBindings: ModelBinding[] = []): void {
@@ -277,6 +281,10 @@ export default class Component {
         // then create a fresh Promise, so any future .then() apply to it
         this.resetPromise();
 
+        // any fields that were modified will now be sent on this request:
+        // they are now "in sync" (with some exceptions noted inside)
+        this.unsyncedInputsTracker.resetUnsyncedFields();
+
         this.backendRequest = this.backend.makeRequest(
             this.valueStore.all(),
             this.pendingActions,

src/LiveComponent/assets/src/morphdom.ts

@@ -12,7 +12,7 @@ import Component from './Component';
 export function executeMorphdom(
     rootFromElement: HTMLElement,
     rootToElement: HTMLElement,
-    modifiedElements: Array<HTMLElement>,
+    modifiedFieldElements: Array<HTMLElement>,
     getElementValue: (element: HTMLElement) => any,
     childComponents: Component[],
     findChildComponent: (id: string, element: HTMLElement) => HTMLElement|null,
@@ -57,7 +57,7 @@ export function executeMorphdom(
 
             // if this field's value has been modified since this HTML was
             // requested, set the toEl's value to match the fromEl
-            if (modifiedElements.includes(fromEl)) {
+            if (modifiedFieldElements.includes(fromEl)) {
                 setValueOnElement(toEl, getElementValue(fromEl))
             }
 

src/LiveComponent/assets/test/UnsyncedInputContainer.test.ts

@@ -9,10 +9,10 @@ describe('UnsyncedInputContainer', () => {
         container.add(element1);
         container.add(element2, 'some_model');
 
-        expect(container.all()).toEqual([element1, element2]);
+        expect(container.allUnsyncedInputs()).toEqual([element1, element2]);
     });
 
-    it('markModelAsSynced removes items added to it', () => {
+    it('markModelAsSynced removes unsynced models but not fields', () => {
         const container = new UnsyncedInputContainer();
         const element1 = htmlToElement('<span>element1</span');
         const element2 = htmlToElement('<span>element2</span');
@@ -23,7 +23,7 @@ describe('UnsyncedInputContainer', () => {
 
         container.markModelAsSynced('some_model2');
 
-        expect(container.all()).toEqual([element1, element3]);
+        expect(container.allUnsyncedInputs()).toEqual([element1, element2, element3]);
     });
 
     it('returns modified models via getModifiedModels()', () => {
@@ -36,6 +36,21 @@ describe('UnsyncedInputContainer', () => {
         container.add(element3, 'some_model3');
 
         container.markModelAsSynced('some_model2');
-        expect(container.getModifiedModels()).toEqual(['some_model3'])
+        expect(container.getUnsyncedModelNames()).toEqual(['some_model3'])
+    });
+
+    it('resetUnsyncedFields removes all model fields except those unsynced', () => {
+        const container = new UnsyncedInputContainer();
+        const element1 = htmlToElement('<span>element1</span');
+        const element2 = htmlToElement('<span>element2</span');
+        const element3 = htmlToElement('<span>element3</span');
+        container.add(element1);
+        container.add(element2, 'some_model2');
+        container.add(element3, 'some_model3');
+
+        container.markModelAsSynced('some_model2');
+
+        container.resetUnsyncedFields();
+        expect(container.allUnsyncedInputs()).toEqual([element1, element3]);
     });
 });

src/LiveComponent/assets/test/controller/model.test.ts

@@ -687,6 +687,59 @@ describe('LiveController data-model Tests', () => {
         expect(unmappedTextarea.getAttribute('class')).toEqual('changed-class');
     });
 
+    it('keeps the unsynced value of a model field mapped via a form', async () => {
+        const test = await createTest({
+            comment: 'Live components',
+        }, (data: any) => `
+            <div ${initComponent(data)}>
+                <form data-model>
+                    <textarea name="comment" data-testid="comment">${data.comment}</textarea>
+               </form>
+
+               <button data-action="live#$render">Reload</button>
+           </div>
+       `);
+
+        test.expectsAjaxCall('get')
+            .expectSentData(test.initialData)
+            .serverWillChangeData((data) => {
+                data.comment = 'server tries to change comment, but it will be modified client side';
+            })
+            // delay slightly so we can type in the textarea
+            .delayResponse(10)
+            .init();
+
+        getByText(test.element, 'Reload').click();
+        // mimic changing the field, but without (yet) triggering the change event
+        const commentField = getByTestId(test.element, 'comment');
+        if (!(commentField instanceof HTMLTextAreaElement)) {
+            throw new Error('wrong type');
+        }
+        userEvent.type(commentField, ' ftw!');
+
+        // wait for loading start and end
+        await waitFor(() => expect(test.element).toHaveAttribute('busy'));
+        await waitFor(() => expect(test.element).not.toHaveAttribute('busy'));
+
+        expect(commentField).toHaveValue('Live components ftw!');
+
+        // refresh again, the value should now be in sync and accept the changed
+        // value from the server
+        test.expectsAjaxCall('get')
+            .expectSentData({ comment: 'Live components ftw!' })
+            .serverWillChangeData((data) => {
+                data.comment = 'server changed comment';
+            })
+            .init();
+
+        getByText(test.element, 'Reload').click();
+        // wait for loading start and end
+        await waitFor(() => expect(test.element).toHaveAttribute('busy'));
+        await waitFor(() => expect(test.element).not.toHaveAttribute('busy'));
+
+        expect(commentField).toHaveValue('server changed comment');
+    });
+
     it('allows model fields to be manually set as long as change event is dispatched', async () => {
         const test = await createTest({ food: '' }, (data: any) => `
             <div ${initComponent(data)}>