Skip to content

Commit

Permalink
feature #1255 Migrate LiveComponent to Idiomorph (matheo, WebMamba)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
weaverryan committed Dec 17, 2023
2 parents 8fe391d + d5fc3c3 commit 98488f7
Show file tree
Hide file tree
Showing 9 changed files with 916 additions and 857 deletions.
9 changes: 8 additions & 1 deletion src/Autocomplete/assets/dist/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/LiveComponent/assets/dist/Component/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1,558 changes: 792 additions & 766 deletions src/LiveComponent/assets/dist/live_controller.js

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/LiveComponent/assets/dist/morphdom.d.ts
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion src/LiveComponent/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
}
},
"dependencies": {
"morphdom": "^2.6.1"
"idiomorph": "^0.0.9"
},
"peerDependencies": {
"@hotwired/stimulus": "^3.0.0"
Expand Down
6 changes: 4 additions & 2 deletions src/LiveComponent/assets/src/Component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -309,6 +309,8 @@ export default class Component {
if (isChanged) {
this.render();
}

return isChanged;
}

/**
Expand Down
183 changes: 103 additions & 80 deletions src/LiveComponent/assets/src/morphdom.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);
});
}
2 changes: 1 addition & 1 deletion src/LiveComponent/assets/test/controller/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 98488f7

Please sign in to comment.