bug #418 [LiveComponent][Autocomplete] Adding support for libraries to play nicely (weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[LiveComponent][Autocomplete] Adding support for libraries to play nicely

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no (but yes to BC break)
| Tickets       | Fixes #407 and helps with #354
| License       | MIT

See the commit for more information - this was a VERY pesky bug.

Cheers!

Commits
-------

c4791fc [LiveComponent][Autocomplete] Adding support for libraries to play nicely together

Author: weaverryan

src/Autocomplete/assets/dist/controller.js

@@ -28,10 +28,16 @@ class default_1 extends Controller {
         super(...arguments);
         _instances.add(this);
     }
-    connect() {
-        if (this.tomSelect) {
-            return;
+    initialize() {
+        this.element.setAttribute('data-live-ignore', '');
+        if (this.element.id) {
+            const label = document.querySelector(`label[for="${this.element.id}"]`);
+            if (label) {
+                label.setAttribute('data-live-ignore', '');
+            }
         }
+    }
+    connect() {
         if (this.urlValue) {
             this.tomSelect = __classPrivateFieldGet(this, _instances, "m", _createAutocompleteWithRemoteData).call(this, this.urlValue);
             return;
@@ -43,6 +49,7 @@ class default_1 extends Controller {
         this.tomSelect = __classPrivateFieldGet(this, _instances, "m", _createAutocomplete).call(this);
     }
     disconnect() {
+        this.tomSelect.revertSettings.innerHTML = this.element.innerHTML;
         this.tomSelect.destroy();
     }
     get selectElement() {
@@ -80,6 +87,10 @@ _instances = new WeakSet(), _getCommonConfig = function _getCommonConfig() {
         onItemAdd: () => {
             this.tomSelect.setTextboxValue('');
         },
+        onInitialize: function () {
+            const tomSelect = this;
+            tomSelect.wrapper.setAttribute('data-live-ignore', '');
+        },
         closeAfterSelect: true,
     };
     if (!this.selectElement && !this.urlValue) {

src/Autocomplete/assets/src/controller.ts

@@ -18,12 +18,17 @@ export default class extends Controller {
     readonly tomSelectOptionsValue: object;
     tomSelect: TomSelect;
 
-    connect() {
-        // this avoids initializing the same field twice (TomSelect shows an error otherwise)
-        if (this.tomSelect) {
-            return;
+    initialize() {
+        this.element.setAttribute('data-live-ignore', '');
+        if (this.element.id) {
+            const label = document.querySelector(`label[for="${this.element.id}"]`);
+            if (label) {
+                label.setAttribute('data-live-ignore', '');
+            }
         }
+    }
 
+    connect() {
         if (this.urlValue) {
             this.tomSelect = this.#createAutocompleteWithRemoteData(this.urlValue);
 
@@ -40,9 +45,9 @@ export default class extends Controller {
     }
 
     disconnect() {
+        // make sure it will "revert" to the latest innerHTML
+        this.tomSelect.revertSettings.innerHTML = this.element.innerHTML;
         this.tomSelect.destroy();
-        // Fixes https://github.com/symfony/ux/issues/407
-        this.tomSelect = undefined;
     }
 
     #getCommonConfig(): Partial<TomSettings> {
@@ -73,6 +78,10 @@ export default class extends Controller {
             onItemAdd: () => {
                 this.tomSelect.setTextboxValue('');
             },
+            onInitialize: function() {
+                const tomSelect = this as any;
+                tomSelect.wrapper.setAttribute('data-live-ignore', '');
+            },
             closeAfterSelect: true,
         };
 
@@ -124,7 +133,7 @@ export default class extends Controller {
             },
             // VERY IMPORTANT: use 'function (query, callback) { ... }' instead of the
             // '(query, callback) => { ... }' syntax because, otherwise,
-            // the 'this.XXX' calls inside of this method fail
+            // the 'this.XXX' calls inside this method fail
             load: function (query: string, callback: (results?: any) => void) {
                 const url = this.getUrl(query);
                 fetch(url)

src/Autocomplete/assets/test/controller.test.ts

@@ -10,7 +10,7 @@
 'use strict';
 
 import { Application, Controller } from '@hotwired/stimulus';
-import { getByTestId, waitFor, getAllByLabelText } from '@testing-library/dom';
+import { getByTestId, waitFor } from '@testing-library/dom';
 import { clearDOM, mountDOM } from '@symfony/stimulus-testing';
 import AutocompleteController from '../src/controller';
 import fetchMock from 'fetch-mock-jest';
@@ -137,4 +137,32 @@ describe('AutocompleteController', () => {
             expect(container.querySelectorAll('.option[data-selectable]')).toHaveLength(2);
         });
     });
+
+    it('adds live-component support', async () => {
+        const container = mountDOM(`
+            <div>
+                <label for="the-select" data-testid="main-element-label">Select something</label>
+                <select
+                    id="the-select"
+                    data-testid="main-element"
+                    data-controller="check autocomplete"
+                ></select>
+            </div>
+        `);
+
+        application = startStimulus();
+
+        await waitFor(() => {
+            expect(getByTestId(container, 'main-element')).toHaveClass('connected');
+        });
+
+        expect(getByTestId(container, 'main-element')).toHaveAttribute('data-live-ignore');
+        expect(getByTestId(container, 'main-element-label')).toHaveAttribute('data-live-ignore');
+        const tsDropdown = container.querySelector('.ts-wrapper');
+
+        await waitFor(() => {
+            expect(tsDropdown).not.toBeNull();
+        });
+        expect(tsDropdown).toHaveAttribute('data-live-ignore');
+    });
 });

src/LiveComponent/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## 2.3.1
 
+-   [BC BREAK] Previously, the `id` attribute was used with `morphdom` as the
+    "node id" when updating the DOM after a render. This has changed to
+    `data-live-id`. This is useful when maintaining the correct order of a list
+    of elements.
+
 -   [BEHAVIOR CHANGE] If an action Ajax call is still processing and a
     model update occurs, the component will _no_ longer re-render. The
     model will be updated internally, but not re-rendered (so, any

src/LiveComponent/assets/dist/live_controller.js

@@ -1599,6 +1599,12 @@ class default_1 extends Controller {
     _executeMorphdom(newHtml, modifiedElements) {
         const newElement = htmlToElement(newHtml);
         morphdom(this.element, newElement, {
+            getNodeKey: (node) => {
+                if (!(node instanceof HTMLElement)) {
+                    return;
+                }
+                return node.dataset.liveId;
+            },
             onBeforeElUpdated: (fromEl, toEl) => {
                 if (!(fromEl instanceof HTMLElement) || !(toEl instanceof HTMLElement)) {
                     return false;
@@ -1626,6 +1632,15 @@ class default_1 extends Controller {
                     return false;
                 }
                 return true;
+            },
+            onBeforeNodeDiscarded(node) {
+                if (!(node instanceof HTMLElement)) {
+                    return true;
+                }
+                if (node.hasAttribute('data-live-ignore')) {
+                    return false;
+                }
+                return true;
             }
         });
         this._exposeOriginalData();

src/LiveComponent/assets/src/live_controller.ts

@@ -694,6 +694,13 @@ export default class extends Controller implements LiveController {
     _executeMorphdom(newHtml: string, modifiedElements: Array<HTMLElement>) {
         const newElement = htmlToElement(newHtml);
         morphdom(this.element, newElement, {
+            getNodeKey: (node: Node) => {
+              if (!(node instanceof HTMLElement)) {
+                  return;
+              }
+
+              return node.dataset.liveId;
+            },
             onBeforeElUpdated: (fromEl, toEl) => {
                 if (!(fromEl instanceof HTMLElement) || !(toEl instanceof HTMLElement)) {
                     return false;
@@ -735,6 +742,18 @@ export default class extends Controller implements LiveController {
                     return false;
                 }
 
+                return true;
+            },
+
+            onBeforeNodeDiscarded(node) {
+                if (!(node instanceof HTMLElement)) {
+                    // text element
+                    return true;
+                }
+
+                if (node.hasAttribute('data-live-ignore')) {
+                    return false;
+                }
                 return true;
             }
         });

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

@@ -13,6 +13,7 @@ import { shutdownTest, createTest, initComponent } from '../tools';
 import { createEvent, fireEvent, getByText, waitFor } from '@testing-library/dom';
 import userEvent from '@testing-library/user-event';
 import fetchMock from 'fetch-mock-jest';
+import { htmlToElement } from '../../src/dom_utils';
 
 describe('LiveController rendering Tests', () => {
     afterEach(() => {
@@ -166,6 +167,7 @@ describe('LiveController rendering Tests', () => {
 
         // imitate some JavaScript changing this element
         test.element.querySelector('span')?.setAttribute('data-foo', 'bar');
+        test.element.appendChild(htmlToElement('<div data-live-ignore>I should not be removed</div>'));
 
         test.expectsAjaxCall('get')
             .expectSentData(test.initialData)
@@ -181,6 +183,38 @@ describe('LiveController rendering Tests', () => {
         const ignoreElement = test.element.querySelector('div[data-live-ignore]');
         expect(ignoreElement).not.toBeNull();
         expect(ignoreElement?.outerHTML).toEqual('<div data-live-ignore="">Inside Ignore Name: <span data-foo="bar">Ryan</span></div>');
+        expect(test.element.innerHTML).toContain('I should not be removed');
+    });
+
+    it('if data-live-id changes, data-live-ignore elements ARE re-rendered', async () => {
+        const test = await createTest({ firstName: 'Ryan', containerId: 'original' }, (data: any) => `
+            <div ${initComponent(data)}>
+                <div data-live-id="${data.containerId}">
+                    <div data-live-ignore>Inside Ignore Name: <span>${data.firstName}</span></div>
+                </div>
+                
+                Outside Ignore Name: ${data.firstName}
+
+                <button data-action="live#$render">Reload</button>
+            </div>
+        `);
+
+        test.expectsAjaxCall('get')
+            .expectSentData(test.initialData)
+            .serverWillChangeData((data: any) => {
+                // change the data on the server so the template renders differently
+                data.firstName = 'Kevin';
+                data.containerId = 'updated';
+            })
+            .init();
+
+        getByText(test.element, 'Reload').click();
+
+        await waitFor(() => expect(test.element).toHaveTextContent('Outside Ignore Name: Kevin'));
+        const ignoreElement = test.element.querySelector('div[data-live-ignore]');
+        expect(ignoreElement).not.toBeNull();
+        // check that even the ignored element re-rendered
+        expect(ignoreElement?.outerHTML).toEqual('<div data-live-ignore="">Inside Ignore Name: <span>Kevin</span></div>');
     });
 
     it('cancels a re-render if the page is navigating away', async () => {

src/LiveComponent/src/Resources/doc/index.rst

@@ -1842,7 +1842,7 @@ changes, a child component will re-render even though it was there
 before *and* after the list changed. This can cause that child component
 to lose some state (i.e. it re-renders with its original live props data).
 
-To fix this, add a unique ``id`` attribute to the root component of each
+To fix this, add a unique ``data-live-id`` attribute to the root component of each
 child element. This will helps LiveComponent identify each item in the
 list and correctly determine if a re-render is necessary, or not.
 
@@ -1860,6 +1860,12 @@ To handle this, add the ``data-live-ignore`` attribute to the element:
 
     <input name="favorite_color" data-live-ignore>
 
+.. note::
+
+    To *force* an ignored element to re-render, give its parent element a
+    ``data-live-id`` attribute. During a re-render, if this value changes, all
+    of the children of the element will be re-rendered, even those with ``data-live-ignore``.
+
 Backward Compatibility promise
 ------------------------------