Skip to content

Commit 7f6aa8f

Browse files
smnandresmnandre
committed
[LiveComponent] Refactor elementBelongsToThisComponent
This method is called at multiple place, sometimes before all children are already instanciated and stored in the component map. So let's back to the quickest selector we have: the browser DOM selector :)
1 parent eb8dd83 commit 7f6aa8f

File tree

3 files changed

+90
-102
lines changed

3 files changed

+90
-102
lines changed

‎src/LiveComponent/assets/dist/live_controller.js‎

Lines changed: 78 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -131,82 +131,6 @@ function getElementAsTagText(element) {
131131
: element.outerHTML;
132132
}
133133

134-
let componentMapByElement = new WeakMap();
135-
let componentMapByComponent = new Map();
136-
const registerComponent = (component) => {
137-
componentMapByElement.set(component.element, component);
138-
componentMapByComponent.set(component, component.name);
139-
};
140-
const unregisterComponent = (component) => {
141-
componentMapByElement.delete(component.element);
142-
componentMapByComponent.delete(component);
143-
};
144-
const getComponent = (element) => new Promise((resolve, reject) => {
145-
let count = 0;
146-
const maxCount = 10;
147-
const interval = setInterval(() => {
148-
const component = componentMapByElement.get(element);
149-
if (component) {
150-
clearInterval(interval);
151-
resolve(component);
152-
}
153-
count++;
154-
if (count > maxCount) {
155-
clearInterval(interval);
156-
reject(new Error(`Component not found for element ${getElementAsTagText(element)}`));
157-
}
158-
}, 5);
159-
});
160-
const findComponents = (currentComponent, onlyParents, onlyMatchName) => {
161-
const components = [];
162-
componentMapByComponent.forEach((componentName, component) => {
163-
if (onlyParents && (currentComponent === component || !component.element.contains(currentComponent.element))) {
164-
return;
165-
}
166-
if (onlyMatchName && componentName !== onlyMatchName) {
167-
return;
168-
}
169-
components.push(component);
170-
});
171-
return components;
172-
};
173-
const findChildren = (currentComponent) => {
174-
const children = [];
175-
componentMapByComponent.forEach((componentName, component) => {
176-
if (currentComponent === component) {
177-
return;
178-
}
179-
if (!currentComponent.element.contains(component.element)) {
180-
return;
181-
}
182-
let foundChildComponent = false;
183-
componentMapByComponent.forEach((childComponentName, childComponent) => {
184-
if (foundChildComponent) {
185-
return;
186-
}
187-
if (childComponent === component) {
188-
return;
189-
}
190-
if (childComponent.element.contains(component.element)) {
191-
foundChildComponent = true;
192-
}
193-
});
194-
children.push(component);
195-
});
196-
return children;
197-
};
198-
const findParent = (currentComponent) => {
199-
let parentElement = currentComponent.element.parentElement;
200-
while (parentElement) {
201-
const component = componentMapByElement.get(parentElement);
202-
if (component) {
203-
return component;
204-
}
205-
parentElement = parentElement.parentElement;
206-
}
207-
return null;
208-
};
209-
210134
function getValueFromElement(element, valueStore) {
211135
if (element instanceof HTMLInputElement) {
212136
if (element.type === 'checkbox') {
@@ -328,16 +252,8 @@ function elementBelongsToThisComponent(element, component) {
328252
if (!component.element.contains(element)) {
329253
return false;
330254
}
331-
let foundChildComponent = false;
332-
findChildren(component).forEach((childComponent) => {
333-
if (foundChildComponent) {
334-
return;
335-
}
336-
if (childComponent.element === element || childComponent.element.contains(element)) {
337-
foundChildComponent = true;
338-
}
339-
});
340-
return !foundChildComponent;
255+
const closestLiveComponent = element.closest('[data-controller~="live"]');
256+
return closestLiveComponent === component.element;
341257
}
342258
function cloneHTMLElement(element) {
343259
const newElement = element.cloneNode(true);
@@ -1883,6 +1799,82 @@ class ExternalMutationTracker {
18831799
}
18841800
}
18851801

1802+
let componentMapByElement = new WeakMap();
1803+
let componentMapByComponent = new Map();
1804+
const registerComponent = (component) => {
1805+
componentMapByElement.set(component.element, component);
1806+
componentMapByComponent.set(component, component.name);
1807+
};
1808+
const unregisterComponent = (component) => {
1809+
componentMapByElement.delete(component.element);
1810+
componentMapByComponent.delete(component);
1811+
};
1812+
const getComponent = (element) => new Promise((resolve, reject) => {
1813+
let count = 0;
1814+
const maxCount = 10;
1815+
const interval = setInterval(() => {
1816+
const component = componentMapByElement.get(element);
1817+
if (component) {
1818+
clearInterval(interval);
1819+
resolve(component);
1820+
}
1821+
count++;
1822+
if (count > maxCount) {
1823+
clearInterval(interval);
1824+
reject(new Error(`Component not found for element ${getElementAsTagText(element)}`));
1825+
}
1826+
}, 5);
1827+
});
1828+
const findComponents = (currentComponent, onlyParents, onlyMatchName) => {
1829+
const components = [];
1830+
componentMapByComponent.forEach((componentName, component) => {
1831+
if (onlyParents && (currentComponent === component || !component.element.contains(currentComponent.element))) {
1832+
return;
1833+
}
1834+
if (onlyMatchName && componentName !== onlyMatchName) {
1835+
return;
1836+
}
1837+
components.push(component);
1838+
});
1839+
return components;
1840+
};
1841+
const findChildren = (currentComponent) => {
1842+
const children = [];
1843+
componentMapByComponent.forEach((componentName, component) => {
1844+
if (currentComponent === component) {
1845+
return;
1846+
}
1847+
if (!currentComponent.element.contains(component.element)) {
1848+
return;
1849+
}
1850+
let foundChildComponent = false;
1851+
componentMapByComponent.forEach((childComponentName, childComponent) => {
1852+
if (foundChildComponent) {
1853+
return;
1854+
}
1855+
if (childComponent === component) {
1856+
return;
1857+
}
1858+
if (childComponent.element.contains(component.element)) {
1859+
foundChildComponent = true;
1860+
}
1861+
});
1862+
children.push(component);
1863+
});
1864+
return children;
1865+
};
1866+
const findParent = (currentComponent) => {
1867+
let parentElement = currentComponent.element.parentElement;
1868+
while (parentElement) {
1869+
const component = componentMapByElement.get(parentElement);
1870+
if (component) {
1871+
return component;
1872+
}
1873+
parentElement = parentElement.parentElement;
1874+
}
1875+
return null;
1876+
};
1877+
18861878
class Component {
18871879
constructor(element, name, props, listeners, id, backend, elementDriver) {
18881880
this.fingerprint = '';

‎src/LiveComponent/assets/src/dom_utils.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type ValueStore from './Component/ValueStore';
22
import { type Directive, parseDirectives } from './Directive/directives_parser';
33
import { normalizeModelName } from './string_utils';
44
import type Component from './Component';
5-
import { findChildren } from './ComponentRegistry';
65
import getElementAsTagText from './Util/getElementAsTagText';
76

87
/**
@@ -208,19 +207,9 @@ export function elementBelongsToThisComponent(element: Element, component: Compo
208207
return false;
209208
}
210209

211-
let foundChildComponent = false;
212-
findChildren(component).forEach((childComponent) => {
213-
if (foundChildComponent) {
214-
// return early
215-
return;
216-
}
217-
218-
if (childComponent.element === element || childComponent.element.contains(element)) {
219-
foundChildComponent = true;
220-
}
221-
});
210+
const closestLiveComponent = element.closest('[data-controller~="live"]');
222211

223-
return !foundChildComponent;
212+
return closestLiveComponent === component.element;
224213
}
225214

226215
export function cloneHTMLElement(element: HTMLElement): HTMLElement {

‎src/LiveComponent/assets/test/dom_utils.test.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,11 +271,19 @@ describe('elementBelongsToThisComponent', () => {
271271
expect(elementBelongsToThisComponent(targetElement, component)).toBeFalsy();
272272
});
273273

274-
it('returns true if element lives inside of controller', () => {
275-
const targetElement = htmlToElement('<input name="user[firstName]">');
274+
it('returns true if element lives inside of a div', () => {
275+
const targetElement = htmlToElement('<input name="user[firstName]"/>');
276276
const component = createComponent('<div></div>');
277277
component.element.appendChild(targetElement);
278278

279+
expect(elementBelongsToThisComponent(targetElement, component)).toBeFalsy();
280+
});
281+
282+
it('returns true if element lives inside of live controller', () => {
283+
const targetElement = htmlToElement('<input name="user[firstName]"/>');
284+
const component = createComponent('<div data-controller="live" data-live-name-value="parentLabel"></div>');
285+
component.element.appendChild(targetElement);
286+
279287
expect(elementBelongsToThisComponent(targetElement, component)).toBeTruthy();
280288
});
281289

@@ -287,7 +295,6 @@ describe('elementBelongsToThisComponent', () => {
287295
const component = createComponent('<div class="parent"></div>');
288296
component.element.appendChild(childComponent.element);
289297

290-
//expect(elementBelongsToThisComponent(targetElement, childComponent)).toBeTruthy();
291298
expect(elementBelongsToThisComponent(targetElement, component)).toBeFalsy();
292299
});
293300

0 commit comments

Comments
 (0)