bug #2399 [LiveComponent] Refactor elementBelongsToThisComponent (smn…

…andre) This PR was merged into the 2.x branch. Discussion ---------- [LiveComponent] Refactor elementBelongsToThisComponent | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Issues | Fix #2388 | License | MIT This method is called from multiple places, sometimes before children component are instanciated and stored in the component map. So let's back to the quickest selector we have: the browser DOM selector :) We'll know tomorrow, but it _could_ solve #2388 Commits ------- 7f6aa8f [LiveComponent] Refactor elementBelongsToThisComponent
symfony · Nov 24, 2024 · 3ea19c1 · 3ea19c1
2 parents 9219cdf + 7f6aa8f
commit 3ea19c1
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 102 deletions.
diff --git a/src/LiveComponent/assets/dist/live_controller.js b/src/LiveComponent/assets/dist/live_controller.js
@@ -131,82 +131,6 @@ function getElementAsTagText(element) {
         : element.outerHTML;
 }
 
-let componentMapByElement = new WeakMap();
-let componentMapByComponent = new Map();
-const registerComponent = (component) => {
-    componentMapByElement.set(component.element, component);
-    componentMapByComponent.set(component, component.name);
-};
-const unregisterComponent = (component) => {
-    componentMapByElement.delete(component.element);
-    componentMapByComponent.delete(component);
-};
-const getComponent = (element) => new Promise((resolve, reject) => {
-    let count = 0;
-    const maxCount = 10;
-    const interval = setInterval(() => {
-        const component = componentMapByElement.get(element);
-        if (component) {
-            clearInterval(interval);
-            resolve(component);
-        }
-        count++;
-        if (count > maxCount) {
-            clearInterval(interval);
-            reject(new Error(`Component not found for element ${getElementAsTagText(element)}`));
-        }
-    }, 5);
-});
-const findComponents = (currentComponent, onlyParents, onlyMatchName) => {
-    const components = [];
-    componentMapByComponent.forEach((componentName, component) => {
-        if (onlyParents && (currentComponent === component || !component.element.contains(currentComponent.element))) {
-            return;
-        }
-        if (onlyMatchName && componentName !== onlyMatchName) {
-            return;
-        }
-        components.push(component);
-    });
-    return components;
-};
-const findChildren = (currentComponent) => {
-    const children = [];
-    componentMapByComponent.forEach((componentName, component) => {
-        if (currentComponent === component) {
-            return;
-        }
-        if (!currentComponent.element.contains(component.element)) {
-            return;
-        }
-        let foundChildComponent = false;
-        componentMapByComponent.forEach((childComponentName, childComponent) => {
-            if (foundChildComponent) {
-                return;
-            }
-            if (childComponent === component) {
-                return;
-            }
-            if (childComponent.element.contains(component.element)) {
-                foundChildComponent = true;
-            }
-        });
-        children.push(component);
-    });
-    return children;
-};
-const findParent = (currentComponent) => {
-    let parentElement = currentComponent.element.parentElement;
-    while (parentElement) {
-        const component = componentMapByElement.get(parentElement);
-        if (component) {
-            return component;
-        }
-        parentElement = parentElement.parentElement;
-    }
-    return null;
-};
-
 function getValueFromElement(element, valueStore) {
     if (element instanceof HTMLInputElement) {
         if (element.type === 'checkbox') {
@@ -320,16 +244,8 @@ function elementBelongsToThisComponent(element, component) {
     if (!component.element.contains(element)) {
         return false;
     }
-    let foundChildComponent = false;
-    findChildren(component).forEach((childComponent) => {
-        if (foundChildComponent) {
-            return;
-        }
-        if (childComponent.element === element || childComponent.element.contains(element)) {
-            foundChildComponent = true;
-        }
-    });
-    return !foundChildComponent;
+    const closestLiveComponent = element.closest('[data-controller~="live"]');
+    return closestLiveComponent === component.element;
 }
 function cloneHTMLElement(element) {
     const newElement = element.cloneNode(true);
@@ -1875,6 +1791,82 @@ class ExternalMutationTracker {
     }
 }
 
+let componentMapByElement = new WeakMap();
+let componentMapByComponent = new Map();
+const registerComponent = (component) => {
+    componentMapByElement.set(component.element, component);
+    componentMapByComponent.set(component, component.name);
+};
+const unregisterComponent = (component) => {
+    componentMapByElement.delete(component.element);
+    componentMapByComponent.delete(component);
+};
+const getComponent = (element) => new Promise((resolve, reject) => {
+    let count = 0;
+    const maxCount = 10;
+    const interval = setInterval(() => {
+        const component = componentMapByElement.get(element);
+        if (component) {
+            clearInterval(interval);
+            resolve(component);
+        }
+        count++;
+        if (count > maxCount) {
+            clearInterval(interval);
+            reject(new Error(`Component not found for element ${getElementAsTagText(element)}`));
+        }
+    }, 5);
+});
+const findComponents = (currentComponent, onlyParents, onlyMatchName) => {
+    const components = [];
+    componentMapByComponent.forEach((componentName, component) => {
+        if (onlyParents && (currentComponent === component || !component.element.contains(currentComponent.element))) {
+            return;
+        }
+        if (onlyMatchName && componentName !== onlyMatchName) {
+            return;
+        }
+        components.push(component);
+    });
+    return components;
+};
+const findChildren = (currentComponent) => {
+    const children = [];
+    componentMapByComponent.forEach((componentName, component) => {
+        if (currentComponent === component) {
+            return;
+        }
+        if (!currentComponent.element.contains(component.element)) {
+            return;
+        }
+        let foundChildComponent = false;
+        componentMapByComponent.forEach((childComponentName, childComponent) => {
+            if (foundChildComponent) {
+                return;
+            }
+            if (childComponent === component) {
+                return;
+            }
+            if (childComponent.element.contains(component.element)) {
+                foundChildComponent = true;
+            }
+        });
+        children.push(component);
+    });
+    return children;
+};
+const findParent = (currentComponent) => {
+    let parentElement = currentComponent.element.parentElement;
+    while (parentElement) {
+        const component = componentMapByElement.get(parentElement);
+        if (component) {
+            return component;
+        }
+        parentElement = parentElement.parentElement;
+    }
+    return null;
+};
+
 class Component {
     constructor(element, name, props, listeners, id, backend, elementDriver) {
         this.fingerprint = '';

diff --git a/src/LiveComponent/assets/src/dom_utils.ts b/src/LiveComponent/assets/src/dom_utils.ts
@@ -2,7 +2,6 @@ import type ValueStore from './Component/ValueStore';
 import { type Directive, parseDirectives } from './Directive/directives_parser';
 import { normalizeModelName } from './string_utils';
 import type Component from './Component';
-import { findChildren } from './ComponentRegistry';
 import getElementAsTagText from './Util/getElementAsTagText';
 
 /**
@@ -200,19 +199,9 @@ export function elementBelongsToThisComponent(element: Element, component: Compo
         return false;
     }
 
-    let foundChildComponent = false;
-    findChildren(component).forEach((childComponent) => {
-        if (foundChildComponent) {
-            // return early
-            return;
-        }
-
-        if (childComponent.element === element || childComponent.element.contains(element)) {
-            foundChildComponent = true;
-        }
-    });
+    const closestLiveComponent = element.closest('[data-controller~="live"]');
 
-    return !foundChildComponent;
+    return closestLiveComponent === component.element;
 }
 
 export function cloneHTMLElement(element: HTMLElement): HTMLElement {

diff --git a/src/LiveComponent/assets/test/dom_utils.test.ts b/src/LiveComponent/assets/test/dom_utils.test.ts
@@ -271,11 +271,19 @@ describe('elementBelongsToThisComponent', () => {
         expect(elementBelongsToThisComponent(targetElement, component)).toBeFalsy();
     });
 
-    it('returns true if element lives inside of controller', () => {
-        const targetElement = htmlToElement('<input name="user[firstName]">');
+    it('returns true if element lives inside of a div', () => {
+        const targetElement = htmlToElement('<input name="user[firstName]"/>');
         const component = createComponent('<div></div>');
         component.element.appendChild(targetElement);
 
+        expect(elementBelongsToThisComponent(targetElement, component)).toBeFalsy();
+    });
+
+    it('returns true if element lives inside of live controller', () => {
+        const targetElement = htmlToElement('<input name="user[firstName]"/>');
+        const component = createComponent('<div data-controller="live" data-live-name-value="parentLabel"></div>');
+        component.element.appendChild(targetElement);
+
         expect(elementBelongsToThisComponent(targetElement, component)).toBeTruthy();
     });
 
@@ -287,7 +295,6 @@ describe('elementBelongsToThisComponent', () => {
         const component = createComponent('<div class="parent"></div>');
         component.element.appendChild(childComponent.element);
 
-        //expect(elementBelongsToThisComponent(targetElement, childComponent)).toBeTruthy();
         expect(elementBelongsToThisComponent(targetElement, component)).toBeFalsy();
     });