Skip to content

[Live] Improving child render handling: avoid removing element from DOM when possible #1561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1332,7 +1332,21 @@ const syncAttributes = function (fromEl, toEl) {
}
};
function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements, getElementValue, externalMutationTracker) {
const preservedOriginalElements = [];
const originalElementIdsToSwapAfter = [];
const originalElementsToPreserve = new Map();
const markElementAsNeedingPostMorphSwap = (id, replaceWithClone) => {
const oldElement = originalElementsToPreserve.get(id);
if (!(oldElement instanceof HTMLElement)) {
throw new Error(`Original element with id ${id} not found`);
}
originalElementIdsToSwapAfter.push(id);
if (!replaceWithClone) {
return null;
}
const clonedOldElement = cloneHTMLElement(oldElement);
oldElement.replaceWith(clonedOldElement);
return clonedOldElement;
};
rootToElement.querySelectorAll('[data-live-preserve]').forEach((newElement) => {
const id = newElement.id;
if (!id) {
Expand All @@ -1342,10 +1356,8 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
if (!(oldElement instanceof HTMLElement)) {
throw new Error(`The element with id "${id}" was not found in the original HTML`);
}
const clonedOldElement = cloneHTMLElement(oldElement);
preservedOriginalElements.push(oldElement);
oldElement.replaceWith(clonedOldElement);
newElement.removeAttribute('data-live-preserve');
originalElementsToPreserve.set(id, oldElement);
syncAttributes(newElement, oldElement);
});
Idiomorph.morph(rootFromElement, rootToElement, {
Expand All @@ -1357,6 +1369,17 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
if (fromEl === rootFromElement) {
return true;
}
if (fromEl.id && originalElementsToPreserve.has(fromEl.id)) {
if (fromEl.id === toEl.id) {
return false;
}
const clonedFromEl = markElementAsNeedingPostMorphSwap(fromEl.id, true);
if (!clonedFromEl) {
throw new Error('missing clone');
}
Idiomorph.morph(clonedFromEl, toEl);
return false;
}
if (fromEl instanceof HTMLElement && toEl instanceof HTMLElement) {
if (typeof fromEl.__x !== 'undefined') {
if (!window.Alpine) {
Expand Down Expand Up @@ -1406,19 +1429,24 @@ function executeMorphdom(rootFromElement, rootToElement, modifiedFieldElements,
if (!(node instanceof HTMLElement)) {
return true;
}
if (node.id && originalElementsToPreserve.has(node.id)) {
markElementAsNeedingPostMorphSwap(node.id, false);
return true;
}
if (externalMutationTracker.wasElementAdded(node)) {
return false;
}
return !node.hasAttribute('data-live-ignore');
},
},
});
preservedOriginalElements.forEach((oldElement) => {
const newElement = rootFromElement.querySelector(`#${oldElement.id}`);
if (!(newElement instanceof HTMLElement)) {
throw new Error('Missing preserved element');
originalElementIdsToSwapAfter.forEach((id) => {
const newElement = rootFromElement.querySelector(`#${id}`);
const originalElement = originalElementsToPreserve.get(id);
if (!(newElement instanceof HTMLElement) || !(originalElement instanceof HTMLElement)) {
throw new Error('Missing elements.');
}
newElement.replaceWith(oldElement);
newElement.replaceWith(originalElement);
});
}

Expand Down
97 changes: 80 additions & 17 deletions src/LiveComponent/assets/src/morphdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,48 @@ export function executeMorphdom(
* To handle them, we:
* 1) Create an array of the "current" HTMLElements that match each
* "data-live-preserve" element.
* 2) Replace the "current" elements with clones so that the originals
* aren't modified during the morphing process.
* 3) After the morphing is complete, we find the preserved elements and
* replace them with the originals.
* 2) If we detect that a "current" preserved element is about to be morphed
* (updated or removed), replace the "current" element with a clone so
* the original isn't modified during the morphing process. Mark that
* this element needs to be replaced after the morphing is complete.
* 3) After the morphing is complete, loop over the elements that were
* replaced and swap the original element back into the new position.
*
* This allows Idiomorph to potentially morph the position of the preserved
* elements... but still allowing us to make sure that the element in the
* new position is exactly the original HTMLElement.
*/
const originalElementIdsToSwapAfter: Array<string> = [];
const originalElementsToPreserve = new Map<string, HTMLElement>();

/**
* Called when a preserved element is about to be morphed.
*
* Instead of allowing the original to be morphed, a fake clone
* is created and morphed instead. The original is then marked
* to be replaced after the morph with wherever the final
* matching id element ends up.
*/
const preservedOriginalElements: HTMLElement[] = [];
const markElementAsNeedingPostMorphSwap = (id: string, replaceWithClone: boolean): HTMLElement | null => {
const oldElement = originalElementsToPreserve.get(id);
if (!(oldElement instanceof HTMLElement)) {
throw new Error(`Original element with id ${id} not found`);
}

originalElementIdsToSwapAfter.push(id);
if (!replaceWithClone) {
return null;
}

const clonedOldElement = cloneHTMLElement(oldElement);
oldElement.replaceWith(clonedOldElement);

return clonedOldElement;
};

rootToElement.querySelectorAll('[data-live-preserve]').forEach((newElement) => {
const id = newElement.id;

if (!id) {
throw new Error('The data-live-preserve attribute requires an id attribute to be set on the element');
}
Expand All @@ -44,11 +78,8 @@ export function executeMorphdom(
throw new Error(`The element with id "${id}" was not found in the original HTML`);
}

const clonedOldElement = cloneHTMLElement(oldElement);
preservedOriginalElements.push(oldElement);
oldElement.replaceWith(clonedOldElement);

newElement.removeAttribute('data-live-preserve');
originalElementsToPreserve.set(id, oldElement);
syncAttributes(newElement, oldElement);
});

Expand All @@ -64,6 +95,27 @@ export function executeMorphdom(
return true;
}

if (fromEl.id && originalElementsToPreserve.has(fromEl.id)) {
if (fromEl.id === toEl.id) {
// the preserved elements match, prevent morph and
// keep the original element
return false;
}

// a preserved element is being morphed into something else
// this means that preserved element is being moved
// to avoid the original element being morphed, we swap
// it for a clone, manually morph the clone, and then
// skip trying to morph the original element (we want it untouched)
const clonedFromEl = markElementAsNeedingPostMorphSwap(fromEl.id, true);
if (!clonedFromEl) {
throw new Error('missing clone');
}
Idiomorph.morph(clonedFromEl, toEl);

return false;
}

// 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.
Expand Down Expand Up @@ -168,6 +220,18 @@ export function executeMorphdom(
return true;
}

if (node.id && originalElementsToPreserve.has(node.id)) {
// a preserved element is being removed
// to avoid the original element being destroyed (but still
// allowing this spot on the dom to be removed),
// clone the original element and place it into the
// new position after morphing
markElementAsNeedingPostMorphSwap(node.id, false);

// allow this to be morphed to the new 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;
Expand All @@ -178,14 +242,13 @@ export function executeMorphdom(
},
});

preservedOriginalElements.forEach((oldElement) => {
const newElement = rootFromElement.querySelector(`#${oldElement.id}`);
if (!(newElement instanceof HTMLElement)) {
// should not happen, as preservedOriginalElements is built from
// the new HTML
throw new Error('Missing preserved element');
originalElementIdsToSwapAfter.forEach((id: string) => {
const newElement = rootFromElement.querySelector(`#${id}`);
const originalElement = originalElementsToPreserve.get(id);
if (!(newElement instanceof HTMLElement) || !(originalElement instanceof HTMLElement)) {
// should not happen
throw new Error('Missing elements.');
}

newElement.replaceWith(oldElement);
newElement.replaceWith(originalElement);
});
}
74 changes: 71 additions & 3 deletions src/LiveComponent/assets/test/controller/child.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ import {
shutdownTests,
getComponent,
dataToJsonAttribute,
getStimulusApplication
} from '../tools';
import { getByTestId, waitFor } from '@testing-library/dom';
import userEvent from '@testing-library/user-event';
import { findChildren } from '../../src/ComponentRegistry';
import { Controller } from '@hotwired/stimulus';

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

it('data-live-preserve child in same location is not removed/re-added to the DOM', async () => {
const originalChild = `
<div ${initComponent({}, {id: 'the-child-id'})}>
<div data-controller="track-connect"></div>
Original Child Component
</div>
`;
const updatedChild = `
<div id="the-child-id" data-live-preserve></div>
`;

const test = await createTest({useOriginalChild: true}, (data: any) => `
<div ${initComponent(data)}>
${data.useOriginalChild ? originalChild : updatedChild}
</div>
`);

getStimulusApplication().register('track-connect', class extends Controller {
disconnect() {
this.element.setAttribute('disconnected', '');
}
});

test.expectsAjaxCall()
.serverWillChangeProps((data: any) => {
data.useOriginalChild = false;
});

await test.component.render();
// sanity check that the child is there
expect(test.element).toHaveTextContent('Original Child Component');
// check that the element was never disconnected/removed from the DOM
expect(test.element).not.toContainHTML('disconnected');
});

it('data-live-preserve element moved correctly when position changes and old element morphed into different element', async () => {
const originalChild = `
<div ${initComponent({}, {id: 'the-child-id'})} data-testid="child-component">
<div data-controller="track-connect"></div>
Original Child Component
</div>
`;
const updatedChild = `
<div id="the-child-id" data-live-preserve></div>
`;

// when morphing original -> updated, the outer div (which was the child)
// will be morphed into a normal div
const test = await createTest({useOriginalChild: true}, (data: any) => `
<div ${initComponent(data)}>
${data.useOriginalChild ? originalChild : ''}
${data.useOriginalChild ? '' : `<div class="wrapper">${updatedChild}</div>`}
</div>
`)

test.expectsAjaxCall()
.serverWillChangeProps((data: any) => {
data.useOriginalChild = false;
});

const childElement = getByTestId(test.element, 'child-component');
await test.component.render();
// sanity check that the child is there
expect(test.element).toHaveTextContent('Original Child Component');
expect(test.element).toContainHTML('class="wrapper"');
expect(childElement.parentElement).toHaveClass('wrapper');
});

it('existing child component gets props & triggers re-render', async () => {
const childTemplate = (data: any) => `
<div ${initComponent(
Expand Down Expand Up @@ -327,10 +397,8 @@ describe('Component parent -> child initialization and rendering tests', () => {
.willReturn(childTemplate);

// trigger the parent render, which will trigger the children to re-render
test.component.render();
await test.component.render();

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

// wait for child to start and stop loading
await waitFor(() => expect(getByTestId(test.element, 'child-component-1')).not.toHaveAttribute('busy'));
Expand Down
6 changes: 5 additions & 1 deletion src/LiveComponent/assets/test/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,14 @@ export async function startStimulus(element: string|HTMLElement) {

return {
controller,
element: controllerElement
element: controllerElement,
}
}

export const getStimulusApplication = (): Application => {
return application;
}

const getControllerElement = (container: HTMLElement): HTMLElement => {
if (container.dataset.controller === 'live') {
return container;
Expand Down