[Live] Fixing bug where inputs would not re-render if the value changed

Author: weaverryan

src/LiveComponent/CHANGELOG.md

@@ -2,10 +2,13 @@
 
 ## 2.1.0
 
+-   Added `data-live-ignore` attribute. If included in an element, that element
+    will not be updated on re-render.
+
 -   The Live Component AJAX endpoints now return HTML in all situations
     instead of JSON.
 
--   Send live action arguments to backend
+-   Ability to send live action arguments to backend
 
 ## 2.0.0
 

src/LiveComponent/assets/src/clone_html_element.ts

@@ -0,0 +1,8 @@
+export function cloneHTMLElement(element: HTMLElement): HTMLElement {
+    const newElement = element.cloneNode(true);
+    if (!(newElement instanceof HTMLElement)) {
+        throw new Error('Could not clone element');
+    }
+
+    return newElement;
+}

src/LiveComponent/assets/src/live_controller.ts

@@ -5,6 +5,8 @@ import { combineSpacedArray } from './string_utils';
 import { buildFormData, buildSearchParams } from './http_data_helper';
 import { setDeepData, doesDeepPropertyExist, normalizeModelName } from './set_deep_data';
 import { haveRenderedValuesChanged } from './have_rendered_values_changed';
+import { normalizeAttributesForComparison } from './normalize_attributes_for_comparison';
+import { cloneHTMLElement } from './clone_html_element';
 
 interface ElementLoadingDirectives {
     element: HTMLElement,
@@ -197,11 +199,7 @@ export default class extends Controller {
         const model = element.dataset.model || element.getAttribute('name');
 
         if (!model) {
-            const clonedElement = (element.cloneNode());
-            // helps typescript know this is an HTMLElement
-            if (!(clonedElement instanceof HTMLElement)) {
-                throw new Error('cloneNode() produced incorrect type');
-            }
+            const clonedElement = cloneHTMLElement(element);
 
             throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-model" or "name" attribute set to the model name.`);
         }
@@ -558,7 +556,18 @@ export default class extends Controller {
             onBeforeElUpdated: (fromEl, toEl) => {
                 // https://github.com/patrick-steele-idem/morphdom#can-i-make-morphdom-blaze-through-the-dom-tree-even-faster-yes
                 if (fromEl.isEqualNode(toEl)) {
-                    return false
+                    // the nodes are equal, but the "value" on some might differ
+                    // lets try to quickly compare a bit more deeply
+                    const normalizedFromEl = cloneHTMLElement(fromEl);
+                    normalizeAttributesForComparison(normalizedFromEl);
+
+                    const normalizedToEl = cloneHTMLElement(toEl);
+                    normalizeAttributesForComparison(normalizedToEl);
+
+                    if (normalizedFromEl.isEqualNode(normalizedToEl)) {
+                        // don't bother updating
+                        return false;
+                    }
                 }
 
                 // avoid updating child components: they will handle themselves
@@ -571,6 +580,11 @@ export default class extends Controller {
                     return false;
                 }
 
+                // look for data-live-ignore, and don't update
+                if (fromEl.hasAttribute('data-live-ignore')) {
+                    return false;
+                }
+
                 return true;
             }
         });

src/LiveComponent/assets/src/normalize_attributes_for_comparison.ts

@@ -0,0 +1,18 @@
+/**
+ * Updates an HTML node to represent its underlying data.
+ *
+ * For example, this finds the value property of each underlying node
+ * and sets that onto the value attribute. This is useful to compare
+ * if two nodes are identical.
+ */
+export function normalizeAttributesForComparison(element: HTMLElement): void {
+    if (element.value) {
+        element.setAttribute('value', element.value);
+    } else if (element.hasAttribute('value')) {
+        element.setAttribute('value', '');
+    }
+
+    Array.from(element.children).forEach((child: HTMLElement) => {
+        normalizeAttributesForComparison(child);
+    });
+}

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

@@ -42,6 +42,9 @@ describe('LiveController Action Tests', () => {
 
     afterEach(() => {
         clearDOM();
+        if (!fetchMock.done()) {
+            throw new Error('Mocked requests did not match');
+        }
         fetchMock.reset();
     });
 
@@ -62,16 +65,14 @@ describe('LiveController Action Tests', () => {
         await waitFor(() => expect(element).toHaveTextContent('Comment Saved!'));
         expect(getByLabelText(element, 'Comments:')).toHaveValue('hi weaver');
 
-        fetchMock.done();
-
         expect(postMock.lastOptions().body.get('comments')).toEqual('hi WEAVER');
     });
 
     it('Sends action named args', async () => {
         const data = { comments: 'hi' };
         const { element } = await startStimulus(template(data));
 
-        fetchMock.postOnce('http://localhost/_components/my_component/sendNamedArgs?values=a%3D1%26b%3D2%26c%3D3', {
+        fetchMock.postOnce('http://localhost/_components/my_component/sendNamedArgs?args=a%3D1%26b%3D2%26c%3D3', {
             html: template({ comments: 'hi' }),
         });
 

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

@@ -72,6 +72,9 @@ describe('LiveController parent -> child component tests', () => {
 
     afterEach(() => {
         clearDOM();
+        if (!fetchMock.done()) {
+            throw new Error('Mocked requests did not match');
+        }
         fetchMock.reset();
     });
 

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

@@ -40,6 +40,9 @@ describe('LiveController CSRF Tests', () => {
 
     afterEach(() => {
         clearDOM();
+        if (!fetchMock.done()) {
+            throw new Error('Mocked requests did not match');
+        }
         fetchMock.reset();
     });
 
@@ -56,7 +59,5 @@ describe('LiveController CSRF Tests', () => {
         await waitFor(() => expect(element).toHaveTextContent('Comment Saved!'));
 
         expect(postMock.lastOptions().headers['X-CSRF-TOKEN']).toEqual('123TOKEN');
-
-        fetchMock.done();
     });
 });

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

@@ -34,6 +34,9 @@ describe('LiveController data-model Tests', () => {
 
     afterEach(() => {
         clearDOM();
+        if (!fetchMock.done()) {
+            throw new Error('Mocked requests did not match');
+        }
         fetchMock.reset();
     });
 
@@ -52,9 +55,6 @@ describe('LiveController data-model Tests', () => {
         await waitFor(() => expect(getByLabelText(element, 'Name:')).toHaveValue('Ryan Weaver'));
         expect(controller.dataValue).toEqual({name: 'Ryan Weaver'});
 
-        // assert all calls were done the correct number of times
-        fetchMock.done();
-
         // assert the input is still focused after rendering
         expect(document.activeElement.dataset.model).toEqual('name');
     });
@@ -69,9 +69,6 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(getByLabelText(element, 'Name:')).toHaveValue('Jan'));
         expect(controller.dataValue).toEqual({name: 'Jan'});
-
-        // assert all calls were done the correct number of times
-        fetchMock.done();
     });
 
 
@@ -111,9 +108,6 @@ describe('LiveController data-model Tests', () => {
         await waitFor(() => expect(getByLabelText(element, 'Name:')).toHaveValue('Ryanguy_'));
         expect(controller.dataValue).toEqual({name: 'Ryanguy_'});
 
-        // assert all calls were done the correct number of times
-        fetchMock.done();
-
         // only 1 render should have ultimately occurred
         expect(renderCount).toEqual(1);
     });
@@ -135,9 +129,6 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver'));
         expect(controller.dataValue).toEqual({name: 'Ryan Weaver'});
-
-        // assert all calls were done the correct number of times
-        fetchMock.done();
     });
 
     it('uses data-model when both name and data-model is present', async () => {
@@ -157,8 +148,6 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver'));
         expect(controller.dataValue).toEqual({name: 'Ryan Weaver'});
-
-        fetchMock.done();
     });
 
     it('uses data-value when both value and data-value is present', async () => {
@@ -178,8 +167,6 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(inputElement).toHaveValue('first_name'));
         expect(controller.dataValue).toEqual({name: 'first_name'});
-
-        fetchMock.done();
     });
 
     it('standardizes user[firstName] style models into post.name', async () => {
@@ -211,9 +198,6 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver'));
         expect(controller.dataValue).toEqual({ user: { firstName: 'Ryan Weaver' } });
-
-        // assert all calls were done the correct number of times
-        fetchMock.done();
     });
 
     it('updates correctly when live#update is on a parent element', async () => {
@@ -245,8 +229,6 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver'));
 
-        fetchMock.done();
-
         // assert the input is still focused after rendering
         expect(document.activeElement.getAttribute('name')).toEqual('firstName');
     });
@@ -264,9 +246,6 @@ describe('LiveController data-model Tests', () => {
         await userEvent.type(inputElement, ' WEAVER');
 
         await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver'));
-
-        // assert all calls were done the correct number of times
-        fetchMock.done();
     });
 
     it('data changed on server should be noticed by controller', async () => {
@@ -283,7 +262,5 @@ describe('LiveController data-model Tests', () => {
 
         await waitFor(() => expect(inputElement).toHaveValue('Kevin Bond'));
         expect(controller.dataValue).toEqual({name: 'Kevin Bond'});
-
-        fetchMock.done();
     });
 });

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

@@ -40,6 +40,9 @@ describe('LiveController rendering Tests', () => {
 
     afterEach(() => {
         clearDOM();
+        if (!fetchMock.done()) {
+            throw new Error('Mocked requests did not match');
+        }
         fetchMock.reset();
     });
 
@@ -57,19 +60,46 @@ describe('LiveController rendering Tests', () => {
         expect(controller.dataValue).toEqual({name: 'Kevin'});
     });
 
-    it('conserves values of fields modified after a render request', async () => {
+    it('renders over local value in input', async () => {
         const data = { name: 'Ryan' };
         const { element } = await startStimulus(template(data));
 
         fetchMock.get(
+            // name=Ryan is sent to the server
             'http://localhost/_components/my_component?name=Ryan',
+            // server changes that data
             template({ name: 'Kevin' }),
             { delay: 100 }
         );
-        getByText(element, 'Reload').click();
+        // type into the input that is not bound to a model
         userEvent.type(getByLabelText(element, 'Comments:'), '!!');
+        getByText(element, 'Reload').click();
 
         await waitFor(() => expect(element).toHaveTextContent('Name: Kevin'));
+        // value if unmapped input is reset
+        expect(getByLabelText(element, 'Comments:')).toHaveValue('i like pizza');
+        expect(document.activeElement.name).toEqual('comments');
+    });
+
+    it('conserves values of fields modified after a render request IF data-live-ignore', async () => {
+        const data = { name: 'Ryan' };
+        const { element } = await startStimulus(template(data));
+
+        fetchMock.get(
+            // name=Ryan is sent to the server
+            'http://localhost/_components/my_component?name=Ryan',
+            // server changes that data
+            template({ name: 'Kevin' }),
+            { delay: 100 }
+        );
+        // type into the input that is not bound to a model
+        const input = getByLabelText(element, 'Comments:');
+        input.setAttribute('data-live-ignore', '');
+        userEvent.type(input, '!!');
+        getByText(element, 'Reload').click();
+
+        await waitFor(() => expect(element).toHaveTextContent('Name: Kevin'));
+        // value of unmapped input is NOT reset because of data-live-ignore
         expect(getByLabelText(element, 'Comments:')).toHaveValue('i like pizza!!');
         expect(document.activeElement.name).toEqual('comments');
     });
@@ -88,8 +118,10 @@ describe('LiveController rendering Tests', () => {
             template({ name: 'Kevin' }, true),
             { delay: 100 }
         );
+        const input = getByLabelText(element, 'Comments:');
+        input.setAttribute('data-live-ignore', '');
+        userEvent.type(input, '!!');
         getByText(element, 'Reload').click();
-        userEvent.type(getByLabelText(element, 'Comments:'), '!!');
 
         await waitFor(() => expect(element).toHaveTextContent('Name: Kevin'));
         expect(getByLabelText(element, 'Comments:')).toHaveValue('i like pizza!!');

src/LiveComponent/assets/test/normalize_attributes_for_comparison.test.ts

@@ -0,0 +1,64 @@
+import { normalizeAttributesForComparison } from '../src/normalize_attributes_for_comparison';
+
+const createElement = function(html: string): HTMLElement {
+    const template = document.createElement('template');
+    html = html.trim();
+    template.innerHTML = html;
+
+    const child = template.content.firstChild;
+    if (!child || !(child instanceof HTMLElement)) {
+        throw new Error('Child not found');
+    }
+
+    return child;
+}
+
+describe('normalizeAttributesForComparison', () => {
+    it('makes no changes if value and attribute not set', () => {
+        const element = createElement('<div class="foo"></div>');
+        normalizeAttributesForComparison(element);
+        expect(element.outerHTML)
+            .toEqual('<div class="foo"></div>');
+    });
+
+    it('sets the attribute if the value is present', () => {
+        const element = createElement('<input class="foo">');
+        element.value = 'set value';
+        normalizeAttributesForComparison(element);
+        expect(element.outerHTML)
+            .toEqual('<input class="foo" value="set value">');
+    });
+
+    it('sets the attribute to empty if the value is empty', () => {
+        const element = createElement('<input class="foo" value="starting">');
+        element.value = '';
+        normalizeAttributesForComparison(element);
+        expect(element.outerHTML)
+            .toEqual('<input class="foo" value="">');
+    });
+
+    it('changes the value attribute if value is different', () => {
+        const element = createElement('<input class="foo" value="starting">');
+        element.value = 'changed';
+        normalizeAttributesForComparison(element);
+        expect(element.outerHTML)
+            .toEqual('<input class="foo" value="changed">');
+    });
+
+    it('changes the value attribute on a child', () => {
+        const element = createElement('<div><input id="child" value="original"></div>');
+        element.querySelector('#child').value = 'changed';
+        normalizeAttributesForComparison(element);
+        expect(element.outerHTML)
+            .toEqual('<div><input id="child" value="changed"></div>');
+    });
+
+    it('changes the value on multiple levels', () => {
+        const element = createElement('<div><input id="child" value="original"><div><input id="grand_child"></div></div>');
+        element.querySelector('#child').value = 'changed';
+        element.querySelector('#grand_child').value = 'changed grand child';
+        normalizeAttributesForComparison(element);
+        expect(element.outerHTML)
+            .toEqual('<div><input id="child" value="changed"><div><input id="grand_child" value="changed grand child"></div></div>');
+    });
+});