feature #1255 Migrate LiveComponent to Idiomorph (matheo, WebMamba)

weaverryan · weaverryan · commit 98488f7be3dd · 2023-12-17T11:59:48.000-05:00
This PR was merged into the 2.x branch. Discussion ---------- Migrate LiveComponent to Idiomorph | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | | License | MIT This PR migrate the LiveComponent morph system from `morphdom` to `idiomorph`. I still have 4 tests that are not passing, and I am not sure how to solve it: 1 - `test/controller/child.test.ts > Component parent -> child initialization and rendering tests > replaces old child with new child if the "id" changes` In `idiomorph` they don't use the id the same way `morphdom` does. So this is the expected behavior, I am not sure if we should replicate the old behavior or not? 2 - `test/controller/child.test.ts > Component parent -> child initialization and rendering tests > tracks various children correctly, even if position changes` I think the issue comes from the behavior explained here: https://github.com/bigskysoftware/idiomorph#example-morph. `Idiomorph` is more clever than `morphdom`, and doesn't need to morph the child on every parent morph. 3 - `test/controller/render-with-external-changes.test.ts > LiveController rendering with external changes tests > will not remove an added element` Looks like a similar issue to the problem above.` Idiomorph` doesn't need to morph every child, so the child is not added to the `ExternalMutationTracker`? 4 - `test/controller/render-with-external-changes.test.ts > LiveController rendering with external changes tests > keeps external changes across multiple renders` ... I'm still digging, but ideas are welcome! 😁 Commits ------- d5fc3c3 add missing build files 317d390 rebuild other ux packages 0df4faa resolve prittier errors a9143ab resolve yarn errors e5a187c fix last bugs 3bce29e Migrate LiveComponent to Idiomorph
diff --git a/src/Autocomplete/assets/dist/controller.js b/src/Autocomplete/assets/dist/controller.js
@@ -15,12 +15,19 @@ 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.
 ***************************************************************************** */
+/* global Reflect, Promise, SuppressedError, Symbol */
+
 
 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);
-}
+}
+
+typeof SuppressedError === "function" ? SuppressedError : function (error, suppressed, message) {
+    var e = new Error(message);
+    return e.name = "SuppressedError", e.error = error, e.suppressed = suppressed, e;
+};
 
 var _default_1_instances, _default_1_getCommonConfig, _default_1_createAutocomplete, _default_1_createAutocompleteWithHtmlContents, _default_1_createAutocompleteWithRemoteData, _default_1_stripTags, _default_1_mergeObjects, _default_1_createTomSelect;
 class default_1 extends Controller {
diff --git a/src/LiveComponent/assets/dist/Component/index.d.ts b/src/LiveComponent/assets/dist/Component/index.d.ts
@@ -53,7 +53,7 @@ export default class Component {
     emitSelf(name: string, data: any): void;
     private performEmit;
     private doEmit;
-    updateFromNewElementFromParentRender(toEl: HTMLElement): void;
+    updateFromNewElementFromParentRender(toEl: HTMLElement): boolean;
     onChildComponentModelUpdate(modelName: string, value: any, childComponent: Component): void;
     private isTurboEnabled;
     private tryStartingRequest;
diff --git a/src/LiveComponent/assets/dist/live_controller.js b/src/LiveComponent/assets/dist/live_controller.js
diff --git a/src/LiveComponent/assets/dist/morphdom.d.ts b/src/LiveComponent/assets/dist/morphdom.d.ts
@@ -1,3 +1,4 @@
+import 'idiomorph';
 import Component from './Component';
 import ExternalMutationTracker from './Rendering/ExternalMutationTracker';
 export declare function executeMorphdom(rootFromElement: HTMLElement, rootToElement: HTMLElement, modifiedFieldElements: Array<HTMLElement>, getElementValue: (element: HTMLElement) => any, childComponents: Component[], findChildComponent: (id: string, element: HTMLElement) => HTMLElement | null, getKeyFromElement: (element: HTMLElement) => string | null, externalMutationTracker: ExternalMutationTracker): void;
diff --git a/src/LiveComponent/assets/package.json b/src/LiveComponent/assets/package.json
@@ -26,7 +26,7 @@
         }
     },
     "dependencies": {
-        "morphdom": "^2.6.1"
+        "idiomorph": "^0.0.9"
     },
     "peerDependencies": {
         "@hotwired/stimulus": "^3.0.0"
diff --git a/src/LiveComponent/assets/src/Component/index.ts b/src/LiveComponent/assets/src/Component/index.ts
@@ -289,13 +289,13 @@ export default class Component {
      *
      * @param toEl
      */
-    updateFromNewElementFromParentRender(toEl: HTMLElement): void {
+    updateFromNewElementFromParentRender(toEl: HTMLElement): boolean {
         const props = this.elementDriver.getComponentProps(toEl);
 
         // if no props are on the element, use the existing element completely
         // this means the parent is signaling that the child does not need to be re-rendered
         if (props === null) {
-            return;
+            return false;
         }
 
         // push props directly down onto the value store
@@ -309,6 +309,8 @@ export default class Component {
         if (isChanged) {
             this.render();
         }
+
+        return isChanged;
     }
 
     /**
diff --git a/src/LiveComponent/assets/src/morphdom.ts b/src/LiveComponent/assets/src/morphdom.ts
@@ -1,5 +1,5 @@
 import { cloneHTMLElement, setValueOnElement } from './dom_utils';
-import morphdom from 'morphdom';
+import 'idiomorph';
 import { normalizeAttributesForComparison } from './normalize_attributes_for_comparison';
 import Component from './Component';
 import ExternalMutationTracker from './Rendering/ExternalMutationTracker';
@@ -19,103 +19,126 @@ export function executeMorphdom(
         childComponentMap.set(childComponent.element, childComponent);
     });
 
-    morphdom(rootFromElement, rootToElement, {
-        getNodeKey: (node: Node) => {
-            if (!(node instanceof HTMLElement)) {
-                return;
-            }
-
-            // Pretend an added element has a unique id so that morphdom treats
-            // it like a unique element, causing it to always attempt to remove
-            // it (which we can then prevent) instead of potentially updating
-            // it from an element that was added by the server in the same location.
-            if (externalMutationTracker.wasElementAdded(node)) {
-                return 'added_element_' + Math.random();
-            }
-
-            return getKeyFromElement(node);
-        },
-        onBeforeElUpdated: (fromEl: Element, toEl: Element) => {
-            if (fromEl === rootFromElement) {
-                return true;
-            }
-
-            // skip special checking if this is, for example, an SVG
-            if (fromEl instanceof HTMLElement && toEl instanceof HTMLElement) {
-                // We assume fromEl is an Alpine component if it has `__x` property.
-                // If it's the case, then we should morph `fromEl` to `ToEl` (thanks to https://alpinejs.dev/plugins/morph)
-                // in order to keep the component state and UI in sync.
-                if (typeof fromEl.__x !== 'undefined') {
-                    if (!window.Alpine) {
-                        throw new Error(
-                            'Unable to access Alpine.js though the global window.Alpine variable. Please make sure Alpine.js is loaded before Symfony UX LiveComponent.'
-                        );
+    Idiomorph.morph(rootFromElement, rootToElement, {
+        callbacks: {
+            beforeNodeMorphed: (fromEl: Element, toEl: Element) => {
+                // Idiomorph loop also over Text node
+                if (!(fromEl instanceof Element) || !(toEl instanceof Element)) {
+                    return true;
+                }
+
+                if (fromEl === rootFromElement) {
+                    return true;
+                }
+
+                let idChanged = false;
+                // Track children if data-live-id changed
+                if (fromEl.hasAttribute('data-live-id')) {
+                    if (fromEl.getAttribute('data-live-id') !== toEl.getAttribute('data-live-id')) {
+                        for (const child of fromEl.children) {
+                            child.setAttribute('parent-live-id-changed', '');
+                        }
+                        idChanged = true;
                     }
+                }
 
-                    if (typeof window.Alpine.morph !== 'function') {
-                        throw new Error(
-                            'Unable to access Alpine.js morph function. Please make sure the Alpine.js Morph plugin is installed and loaded, see https://alpinejs.dev/plugins/morph for more information.'
-                        );
+                // skip special checking if this is, for example, an SVG
+                if (fromEl instanceof HTMLElement && toEl instanceof HTMLElement) {
+                    // We assume fromEl is an Alpine component if it has `__x` property.
+                    // If it's the case, then we should morph `fromEl` to `ToEl` (thanks to https://alpinejs.dev/plugins/morph)
+                    // in order to keep the component state and UI in sync.
+                    if (typeof fromEl.__x !== 'undefined') {
+                        if (!window.Alpine) {
+                            throw new Error(
+                                'Unable to access Alpine.js though the global window.Alpine variable. Please make sure Alpine.js is loaded before Symfony UX LiveComponent.'
+                            );
+                        }
+
+                        if (typeof window.Alpine.morph !== 'function') {
+                            throw new Error(
+                                'Unable to access Alpine.js morph function. Please make sure the Alpine.js Morph plugin is installed and loaded, see https://alpinejs.dev/plugins/morph for more information.'
+                            );
+                        }
+
+                        window.Alpine.morph(fromEl.__x, toEl);
                     }
 
-                    window.Alpine.morph(fromEl.__x, toEl);
-                }
+                    if (childComponentMap.has(fromEl)) {
+                        const childComponent = childComponentMap.get(fromEl) as Component;
 
-                if (childComponentMap.has(fromEl)) {
-                    const childComponent = childComponentMap.get(fromEl) as Component;
+                        return !childComponent.updateFromNewElementFromParentRender(toEl) && idChanged;
+                    }
 
-                    childComponent.updateFromNewElementFromParentRender(toEl);
+                    if (externalMutationTracker.wasElementAdded(fromEl)) {
+                        fromEl.insertAdjacentElement('afterend', toEl);
+                        return false;
+                    }
 
-                    return false;
-                }
+                    // if this field's value has been modified since this HTML was
+                    // requested, set the toEl's value to match the fromEl
+                    if (modifiedFieldElements.includes(fromEl)) {
+                        setValueOnElement(toEl, getElementValue(fromEl));
+                    }
 
-                // if this field's value has been modified since this HTML was
-                // requested, set the toEl's value to match the fromEl
-                if (modifiedFieldElements.includes(fromEl)) {
-                    setValueOnElement(toEl, getElementValue(fromEl));
+                    // handle any external changes to this element
+                    const elementChanges = externalMutationTracker.getChangedElement(fromEl);
+                    if (elementChanges) {
+                        // apply the changes to the "to" element so it looks like the
+                        // external changes were already part of it
+                        elementChanges.applyToElement(toEl);
+                    }
+
+                    // https://github.com/patrick-steele-idem/morphdom#can-i-make-morphdom-blaze-through-the-dom-tree-even-faster-yes
+                    if (fromEl.nodeName.toUpperCase() !== 'OPTION' && fromEl.isEqualNode(toEl)) {
+                        // 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;
+                        }
+                    }
                 }
 
-                // handle any external changes to this element
-                const elementChanges = externalMutationTracker.getChangedElement(fromEl);
-                if (elementChanges) {
-                    // apply the changes to the "to" element so it looks like the
-                    // external changes were already part of it
-                    elementChanges.applyToElement(toEl);
+                // Update child if parent has his live-id changed
+                if (fromEl.hasAttribute('parent-live-id-changed')) {
+                    fromEl.removeAttribute('parent-live-id-changed');
+
+                    return true;
                 }
 
-                // https://github.com/patrick-steele-idem/morphdom#can-i-make-morphdom-blaze-through-the-dom-tree-even-faster-yes
-                if (fromEl.nodeName.toUpperCase() !== 'OPTION' && fromEl.isEqualNode(toEl)) {
-                    // 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);
+                // look for data-live-ignore, and don't update
+                return !fromEl.hasAttribute('data-live-ignore');
+            },
 
-                    const normalizedToEl = cloneHTMLElement(toEl);
-                    normalizeAttributesForComparison(normalizedToEl);
+            beforeNodeRemoved(node) {
+                if (!(node instanceof HTMLElement)) {
+                    // text element
+                    return true;
+                }
 
-                    if (normalizedFromEl.isEqualNode(normalizedToEl)) {
-                        // don't bother updating
-                        return false;
-                    }
+                if (externalMutationTracker.wasElementAdded(node)) {
+                    // this element was added by an external mutation, so we don't want to discard it
+                    return false;
                 }
-            }
 
-            // look for data-live-ignore, and don't update
-            return !fromEl.hasAttribute('data-live-ignore');
+                return !node.hasAttribute('data-live-ignore');
+            },
         },
+    });
 
-        onBeforeNodeDiscarded(node) {
-            if (!(node instanceof HTMLElement)) {
-                // text element
-                return true;
-            }
-
-            if (externalMutationTracker.wasElementAdded(node)) {
-                // this element was added by an external mutation, so we don't want to discard it
-                return false;
-            }
+    childComponentMap.forEach((childComponent, element) => {
+        const childComponentInResult = findChildComponent(childComponent.id ?? '', rootFromElement);
+        if (null === childComponentInResult || element === childComponentInResult) {
+            return;
+        }
 
-            return !node.hasAttribute('data-live-ignore');
-        },
+        childComponentInResult?.replaceWith(element);
+        childComponent.updateFromNewElementFromParentRender(childComponentInResult);
     });
 }
diff --git a/src/LiveComponent/assets/test/controller/render.test.ts b/src/LiveComponent/assets/test/controller/render.test.ts
@@ -437,7 +437,7 @@ describe('LiveController rendering Tests', () => {
         const selectOption2 = test.element.querySelector('#select_option_2') as HTMLSelectElement;
 
         // verify the placeholder of the select option 2 is selected
-        expect(selectOption2.children[0].hasAttribute('selected')).toBe(true);
+        expect(selectOption2.children[0].selected).toBe(true);
 
         // verify the selectedIndex of the select option 2 is 0
         expect(selectOption2.selectedIndex).toBe(0);
diff --git a/yarn.lock b/yarn.lock
@@ -5437,6 +5437,11 @@ icss-utils@^5.0.0:
   resolved "https://registry.yarnpkg.com/icss-utils/-/icss-utils-5.1.0.tgz#c6be6858abd013d768e98366ae47e25d5887b1ae"
   integrity sha512-soFhflCVWLfRNOPU3iv5Z9VUdT44xFRbzjLsEzSr5AQmgqPMTHdU3PMT1Cf1ssx8fLNJDA1juftYl+PUcv3MqA==
 
+idiomorph@^0.0.9:
+  version "0.0.9"
+  resolved "https://registry.yarnpkg.com/idiomorph/-/idiomorph-0.0.9.tgz#938d5964031381b0713398fb283aa3754306fa1b"
+  integrity sha512-X7SGsldE5jQD+peLjNLAnIJDEZtGpuLrNRUBrTWMMnzrx9gwy5U+SCRhaifO2v2Z+8j09IY2J+EYaxHsOLTD0A==
+
 ignore@^5.2.0:
   version "5.2.4"
   resolved "https://registry.yarnpkg.com/ignore/-/ignore-5.2.4.tgz#a291c0c6178ff1b960befe47fcdec301674a6324"
@@ -6870,11 +6875,6 @@ moo-color@^1.0.2:
   dependencies:
     color-name "^1.1.4"
 
-morphdom@^2.6.1:
-  version "2.7.0"
-  resolved "https://registry.yarnpkg.com/morphdom/-/morphdom-2.7.0.tgz#9ef0c4bc15ac8725df398d127c6984f62e7f89e8"
-  integrity sha512-8L8DwbdjjWwM/aNqj7BSoSn4G7SQLNiDcxCnMWbf506jojR6lNQ5YOmQqXEIE8u3C492UlkN4d0hQwz97+M1oQ==
-
 mri@^1.1.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/mri/-/mri-1.2.0.tgz#6721480fec2a11a4889861115a48b6cbe7cc8f0b"

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@`
`26`	`26`	`}`
`27`	`27`	`},`
`28`	`28`	`"dependencies": {`
`29`		`- "morphdom": "^2.6.1"`
	`29`	`+ "idiomorph": "^0.0.9"`
`30`	`30`	`},`
`31`	`31`	`"peerDependencies": {`
`32`	`32`	`"@hotwired/stimulus": "^3.0.0"`
Original file line number	Diff line number	Diff line change
`@@ -289,13 +289,13 @@ export default class Component {`
`289`	`289`	`*`
`290`	`290`	`* @param toEl`
`291`	`291`	`*/`
`292`		`- updateFromNewElementFromParentRender(toEl: HTMLElement): void {`
	`292`	`+ updateFromNewElementFromParentRender(toEl: HTMLElement): boolean {`
`293`	`293`	`const props = this.elementDriver.getComponentProps(toEl);`
`294`	`294`
`295`	`295`	`// if no props are on the element, use the existing element completely`
`296`	`296`	`// this means the parent is signaling that the child does not need to be re-rendered`
`297`	`297`	`if (props === null) {`
`298`		`- return;`
	`298`	`+ return false;`
`299`	`299`	`}`
`300`	`300`
`301`	`301`	`// push props directly down onto the value store`
`@@ -309,6 +309,8 @@ export default class Component {`
`309`	`309`	`if (isChanged) {`
`310`	`310`	`this.render();`
`311`	`311`	`}`
	`312`	`+`
	`313`	`+ return isChanged;`
`312`	`314`	`}`
`313`	`315`
`314`	`316`	`/**`