Skip to content

Commit 603d653

Browse files
committed
[Live] Improving child render handling: avoid removing element from DOM when possible
1 parent 1a4f208 commit 603d653

File tree

4 files changed

+193
-30
lines changed

4 files changed

+193
-30
lines changed

src/LiveComponent/assets/dist/live_controller.js

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,7 +1332,21 @@ const syncAttributes = function (fromEl, toEl) {
13321332
}
13331333
};
13341334
function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements, getElementValue, externalMutationTracker) {
1335-
const preservedOriginalElements = [];
1335+
const originalElementIdsToSwapAfter = [];
1336+
const originalElementsToPreserve = new Map();
1337+
const markElementAsNeedingPostMorphSwap = (id, replaceWithClone) => {
1338+
const oldElement = originalElementsToPreserve.get(id);
1339+
if (!(oldElement instanceof HTMLElement)) {
1340+
throw new Error(`Original element with id ${id} not found`);
1341+
}
1342+
originalElementIdsToSwapAfter.push(id);
1343+
if (!replaceWithClone) {
1344+
return null;
1345+
}
1346+
const clonedOldElement = cloneHTMLElement(oldElement);
1347+
oldElement.replaceWith(clonedOldElement);
1348+
return clonedOldElement;
1349+
};
13361350
rootToElement.querySelectorAll('[data-live-preserve]').forEach((newElement) => {
13371351
const id = newElement.id;
13381352
if (!id) {
@@ -1342,10 +1356,8 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
13421356
if (!(oldElement instanceof HTMLElement)) {
13431357
throw new Error(`The element with id "${id}" was not found in the original HTML`);
13441358
}
1345-
const clonedOldElement = cloneHTMLElement(oldElement);
1346-
preservedOriginalElements.push(oldElement);
1347-
oldElement.replaceWith(clonedOldElement);
13481359
newElement.removeAttribute('data-live-preserve');
1360+
originalElementsToPreserve.set(id, oldElement);
13491361
syncAttributes(newElement, oldElement);
13501362
});
13511363
Idiomorph.morph(rootFromElement, rootToElement, {
@@ -1357,6 +1369,17 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
13571369
if (fromEl === rootFromElement) {
13581370
return true;
13591371
}
1372+
if (fromEl.id && originalElementsToPreserve.has(fromEl.id)) {
1373+
if (fromEl.id === toEl.id) {
1374+
return false;
1375+
}
1376+
const clonedFromEl = markElementAsNeedingPostMorphSwap(fromEl.id, true);
1377+
if (!clonedFromEl) {
1378+
throw new Error('missing clone');
1379+
}
1380+
Idiomorph.morph(clonedFromEl, toEl);
1381+
return false;
1382+
}
13601383
if (fromEl instanceof HTMLElement && toEl instanceof HTMLElement) {
13611384
if (typeof fromEl.__x !== 'undefined') {
13621385
if (!window.Alpine) {
@@ -1406,19 +1429,24 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
14061429
if (!(node instanceof HTMLElement)) {
14071430
return true;
14081431
}
1432+
if (node.id && originalElementsToPreserve.has(node.id)) {
1433+
markElementAsNeedingPostMorphSwap(node.id, false);
1434+
return true;
1435+
}
14091436
if (externalMutationTracker.wasElementAdded(node)) {
14101437
return false;
14111438
}
14121439
return !node.hasAttribute('data-live-ignore');
14131440
},
14141441
},
14151442
});
1416-
preservedOriginalElements.forEach((oldElement) => {
1417-
const newElement = rootFromElement.querySelector(`#${oldElement.id}`);
1418-
if (!(newElement instanceof HTMLElement)) {
1419-
throw new Error('Missing preserved element');
1443+
originalElementIdsToSwapAfter.forEach((id) => {
1444+
const newElement = rootFromElement.querySelector(`#${id}`);
1445+
const originalElement = originalElementsToPreserve.get(id);
1446+
if (!(newElement instanceof HTMLElement) || !(originalElement instanceof HTMLElement)) {
1447+
throw new Error('Missing elements.');
14201448
}
1421-
newElement.replaceWith(oldElement);
1449+
newElement.replaceWith(originalElement);
14221450
});
14231451
}
14241452

src/LiveComponent/assets/src/morphdom.ts

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,48 @@ export function executeMorphdom(
2727
* To handle them, we:
2828
* 1) Create an array of the "current" HTMLElements that match each
2929
* "data-live-preserve" element.
30-
* 2) Replace the "current" elements with clones so that the originals
31-
* aren't modified during the morphing process.
32-
* 3) After the morphing is complete, we find the preserved elements and
33-
* replace them with the originals.
30+
* 2) If we detect that a "current" preserved element is about to be morphed
31+
* (updated or removed), replace the "current" element with a clone so
32+
* the original isn't modified during the morphing process. Mark that
33+
* this element needs to be replaced after the morphing is complete.
34+
* 3) After the morphing is complete, loop over the elements that were
35+
* replaced and swap the original element back into the new position.
36+
*
37+
* This allows Idiomorph to potentially morph the position of the preserved
38+
* elements... but still allowing us to make sure that the element in the
39+
* new position is exactly the original HTMLElement.
40+
*/
41+
const originalElementIdsToSwapAfter: Array<string> = [];
42+
const originalElementsToPreserve = new Map<string, HTMLElement>();
43+
44+
/**
45+
* Called when a preserved element is about to be morphed.
46+
*
47+
* Instead of allowing the original to be morphed, a fake clone
48+
* is created and morphed instead. The original is then marked
49+
* to be replaced after the morph with wherever the final
50+
* matching id element ends up.
3451
*/
35-
const preservedOriginalElements: HTMLElement[] = [];
52+
const markElementAsNeedingPostMorphSwap = (id: string, replaceWithClone: boolean): HTMLElement | null => {
53+
const oldElement = originalElementsToPreserve.get(id);
54+
if (!(oldElement instanceof HTMLElement)) {
55+
throw new Error(`Original element with id ${id} not found`);
56+
}
57+
58+
originalElementIdsToSwapAfter.push(id);
59+
if (!replaceWithClone) {
60+
return null;
61+
}
62+
63+
const clonedOldElement = cloneHTMLElement(oldElement);
64+
oldElement.replaceWith(clonedOldElement);
65+
66+
return clonedOldElement;
67+
};
68+
3669
rootToElement.querySelectorAll('[data-live-preserve]').forEach((newElement) => {
3770
const id = newElement.id;
71+
3872
if (!id) {
3973
throw new Error('The data-live-preserve attribute requires an id attribute to be set on the element');
4074
}
@@ -44,11 +78,8 @@ export function executeMorphdom(
4478
throw new Error(`The element with id "${id}" was not found in the original HTML`);
4579
}
4680

47-
const clonedOldElement = cloneHTMLElement(oldElement);
48-
preservedOriginalElements.push(oldElement);
49-
oldElement.replaceWith(clonedOldElement);
50-
5181
newElement.removeAttribute('data-live-preserve');
82+
originalElementsToPreserve.set(id, oldElement);
5283
syncAttributes(newElement, oldElement);
5384
});
5485

@@ -64,6 +95,27 @@ export function executeMorphdom(
6495
return true;
6596
}
6697

98+
if (fromEl.id && originalElementsToPreserve.has(fromEl.id)) {
99+
if (fromEl.id === toEl.id) {
100+
// the preserved elements match, prevent morph and
101+
// keep the original element
102+
return false;
103+
}
104+
105+
// a preserved element is being morphed into something else
106+
// this means that preserved element is being moved
107+
// to avoid the original element being morphed, we swap
108+
// it for a clone, manually morph the clone, and then
109+
// skip trying to morph the original element (we want it untouched)
110+
const clonedFromEl = markElementAsNeedingPostMorphSwap(fromEl.id, true);
111+
if (!clonedFromEl) {
112+
throw new Error('missing clone');
113+
}
114+
Idiomorph.morph(clonedFromEl, toEl);
115+
116+
return false;
117+
}
118+
67119
// skip special checking if this is, for example, an SVG
68120
if (fromEl instanceof HTMLElement && toEl instanceof HTMLElement) {
69121
// We assume fromEl is an Alpine component if it has `__x` property.
@@ -168,6 +220,18 @@ export function executeMorphdom(
168220
return true;
169221
}
170222

223+
if (node.id && originalElementsToPreserve.has(node.id)) {
224+
// a preserved element is being removed
225+
// to avoid the original element being destroyed (but still
226+
// allowing this spot on the dom to be removed),
227+
// clone the original element and place it into the
228+
// new position after morphing
229+
markElementAsNeedingPostMorphSwap(node.id, false);
230+
231+
// allow this to be morphed to the new element
232+
return true;
233+
}
234+
171235
if (externalMutationTracker.wasElementAdded(node)) {
172236
// this element was added by an external mutation, so we don't want to discard it
173237
return false;
@@ -178,14 +242,13 @@ export function executeMorphdom(
178242
},
179243
});
180244

181-
preservedOriginalElements.forEach((oldElement) => {
182-
const newElement = rootFromElement.querySelector(`#${oldElement.id}`);
183-
if (!(newElement instanceof HTMLElement)) {
184-
// should not happen, as preservedOriginalElements is built from
185-
// the new HTML
186-
throw new Error('Missing preserved element');
245+
originalElementIdsToSwapAfter.forEach((id: string) => {
246+
const newElement = rootFromElement.querySelector(`#${id}`);
247+
const originalElement = originalElementsToPreserve.get(id);
248+
if (!(newElement instanceof HTMLElement) || !(originalElement instanceof HTMLElement)) {
249+
// should not happen
250+
throw new Error('Missing elements.');
187251
}
188-
189-
newElement.replaceWith(oldElement);
252+
newElement.replaceWith(originalElement);
190253
});
191254
}

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

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ import {
1616
shutdownTests,
1717
getComponent,
1818
dataToJsonAttribute,
19+
getStimulusApplication
1920
} from '../tools';
2021
import { getByTestId, waitFor } from '@testing-library/dom';
2122
import userEvent from '@testing-library/user-event';
2223
import { findChildren } from '../../src/ComponentRegistry';
24+
import { Controller } from '@hotwired/stimulus';
2325

2426
describe('Component parent -> child initialization and rendering tests', () => {
2527
afterEach(() => {
@@ -128,6 +130,74 @@ describe('Component parent -> child initialization and rendering tests', () => {
128130
expect(test.element).not.toContainHTML('data-live-preserve');
129131
});
130132

133+
it('data-live-preserve child in same location is not removed/re-added to the DOM', async () => {
134+
const originalChild = `
135+
<div ${initComponent({}, {id: 'the-child-id'})}>
136+
<div data-controller="track-connect"></div>
137+
Original Child Component
138+
</div>
139+
`;
140+
const updatedChild = `
141+
<div id="the-child-id" data-live-preserve></div>
142+
`;
143+
144+
const test = await createTest({useOriginalChild: true}, (data: any) => `
145+
<div ${initComponent(data)}>
146+
${data.useOriginalChild ? originalChild : updatedChild}
147+
</div>
148+
`);
149+
150+
getStimulusApplication().register('track-connect', class extends Controller {
151+
disconnect() {
152+
this.element.setAttribute('disconnected', '');
153+
}
154+
});
155+
156+
test.expectsAjaxCall()
157+
.serverWillChangeProps((data: any) => {
158+
data.useOriginalChild = false;
159+
});
160+
161+
await test.component.render();
162+
// sanity check that the child is there
163+
expect(test.element).toHaveTextContent('Original Child Component');
164+
// check that the element was never disconnected/removed from the DOM
165+
expect(test.element).not.toContainHTML('disconnected');
166+
});
167+
168+
it('data-live-preserve element moved correctly when position changes and old element morphed into different element', async () => {
169+
const originalChild = `
170+
<div ${initComponent({}, {id: 'the-child-id'})} data-testid="child-component">
171+
<div data-controller="track-connect"></div>
172+
Original Child Component
173+
</div>
174+
`;
175+
const updatedChild = `
176+
<div id="the-child-id" data-live-preserve></div>
177+
`;
178+
179+
// when morphing original -> updated, the outer div (which was the child)
180+
// will be morphed into a normal div
181+
const test = await createTest({useOriginalChild: true}, (data: any) => `
182+
<div ${initComponent(data)}>
183+
${data.useOriginalChild ? originalChild : ''}
184+
${data.useOriginalChild ? '' : `<div class="wrapper">${updatedChild}</div>`}
185+
</div>
186+
`)
187+
188+
test.expectsAjaxCall()
189+
.serverWillChangeProps((data: any) => {
190+
data.useOriginalChild = false;
191+
});
192+
193+
const childElement = getByTestId(test.element, 'child-component');
194+
await test.component.render();
195+
// sanity check that the child is there
196+
expect(test.element).toHaveTextContent('Original Child Component');
197+
expect(test.element).toContainHTML('class="wrapper"');
198+
expect(childElement.parentElement).toHaveClass('wrapper');
199+
});
200+
131201
it('existing child component gets props & triggers re-render', async () => {
132202
const childTemplate = (data: any) => `
133203
<div ${initComponent(
@@ -327,10 +397,8 @@ describe('Component parent -> child initialization and rendering tests', () => {
327397
.willReturn(childTemplate);
328398

329399
// trigger the parent render, which will trigger the children to re-render
330-
test.component.render();
400+
await test.component.render();
331401

332-
// wait for parent Ajax call to finish
333-
await waitFor(() => expect(test.element).not.toHaveAttribute('busy'));
334402

335403
// wait for child to start and stop loading
336404
await waitFor(() => expect(getByTestId(test.element, 'child-component-1')).not.toHaveAttribute('busy'));

src/LiveComponent/assets/test/tools.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,10 +394,14 @@ export async function startStimulus(element: string|HTMLElement) {
394394

395395
return {
396396
controller,
397-
element: controllerElement
397+
element: controllerElement,
398398
}
399399
}
400400

401+
export const getStimulusApplication = (): Application => {
402+
return application;
403+
}
404+
401405
const getControllerElement = (container: HTMLElement): HTMLElement => {
402406
if (container.dataset.controller === 'live') {
403407
return container;

0 commit comments

Comments
 (0)