Skip to content
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

Migrate LiveComponent to Idiomorph #1255

Merged
merged 6 commits into from
Dec 17, 2023
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
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 @@ -285,13 +285,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 @@ -305,6 +305,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';
weaverryan marked this conversation as resolved.
Show resolved Hide resolved
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this is now needed? I see some slight change in the logic above, but I can't quite understand the significance. Also, for reference, I'm checking the diff with https://github.com/symfony/ux/pull/1255/files?w=1 so that it hides whitespace differences and it's easier for me to see what actually changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho man! This PR looks so far away now! But I think this is related to the last failing test. The thing is idiomorph is more clever than morphdom so when you have a component that has children and the children. If your component rerender and the children swap position idiopmorph is clever enough to just morph the position of the children. But the thing is this is so clever that the child is not rerender. So here we forced the children to be rerender in this situation. I hope this help

});
}
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 @@ -5366,6 +5366,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 @@ -6782,11 +6787,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
Loading