From f11b067c2e441264f5614d3ecddbf7242a3d84b1 Mon Sep 17 00:00:00 2001 From: Nayden Naydenov Date: Tue, 25 Feb 2025 13:10:42 +0200 Subject: [PATCH 1/4] fix(framework): invalidation of unchanged slots --- .../base/cypress/specs/UI5ElementSlots.cy.tsx | 34 +++++++++++++++++++ packages/base/src/UI5Element.ts | 18 ++-------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/packages/base/cypress/specs/UI5ElementSlots.cy.tsx b/packages/base/cypress/specs/UI5ElementSlots.cy.tsx index 8be14d185cc1..0235e49a6669 100644 --- a/packages/base/cypress/specs/UI5ElementSlots.cy.tsx +++ b/packages/base/cypress/specs/UI5ElementSlots.cy.tsx @@ -133,4 +133,38 @@ describe("Slots work properly", () => { .invoke("prop", "items") .should("have.length", 3); }); + + it("Tests that only changed slot triggers invalidation", () => { + cy.mount( + + Default slot content + Other slot content 2 + + ); + + cy.get("[ui5-test-generic]") + .as("testGeneric"); + + cy.get("@testGeneric") + .should("be.visible"); + + cy.get("@testGeneric") + .then($el => { + cy.stub($el[0], "onInvalidation").as("invalidation"); + }); + + cy.get("@invalidation") + .should("not.have.been.called"); + + cy.get("@testGeneric") + .then($el => { + const newEl = document.createElement("span"); + newEl.innerText = "New Element"; + + $el.append(newEl); + }); + + cy.get("@invalidation") + .should("have.been.calledOnce"); + }); }); diff --git a/packages/base/src/UI5Element.ts b/packages/base/src/UI5Element.ts index d2e2d0b567d1..702b5336c545 100644 --- a/packages/base/src/UI5Element.ts +++ b/packages/base/src/UI5Element.ts @@ -235,21 +235,6 @@ abstract class UI5Element extends HTMLElement { if (ctor._needsShadowDOM()) { const defaultOptions = { mode: "open" } as ShadowRootInit; this.attachShadow({ ...defaultOptions, ...ctor.getMetadata().getShadowRootOptions() }); - - const slotsAreManaged = ctor.getMetadata().slotsAreManaged(); - if (slotsAreManaged) { - this.shadowRoot!.addEventListener("slotchange", this._onShadowRootSlotChange.bind(this)); - } - } - } - - /** - * Note: this "slotchange" listener is for slots, rendered in the component's shadow root - */ - _onShadowRootSlotChange(e: Event) { - const targetShadowRoot = (e.target as Node)?.getRootNode(); // the "slotchange" event target is always a slot element - if (targetShadowRoot === this.shadowRoot) { // only for slotchange events that originate from slots, belonging to the component's shadow root - this._processChildren(); } } @@ -380,10 +365,11 @@ abstract class UI5Element extends HTMLElement { } const canSlotText = metadata.canSlotText(); - const mutationObserverOptions = { + const mutationObserverOptions: MutationObserverInit = { childList: true, subtree: canSlotText, characterData: canSlotText, + attributeFilter: ["slot"], }; observeDOMNode(this, this._processChildren.bind(this) as MutationCallback, mutationObserverOptions); } From ad577a2abc0a1e0b105268cff52b06843259dd08 Mon Sep 17 00:00:00 2001 From: Nayden Naydenov Date: Wed, 26 Feb 2025 09:55:17 +0200 Subject: [PATCH 2/4] chore: fix build --- packages/base/src/UI5Element.ts | 48 ++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/packages/base/src/UI5Element.ts b/packages/base/src/UI5Element.ts index 702b5336c545..86163b6f855c 100644 --- a/packages/base/src/UI5Element.ts +++ b/packages/base/src/UI5Element.ts @@ -138,20 +138,20 @@ function getPropertyDescriptor(proto: any, name: PropertyKey): PropertyDescripto type NotEqual = true extends Equal ? false : true type Equal = - (() => T extends X ? 1 : 2) extends - (() => T extends Y ? 1 : 2) ? true : false + (() => T extends X ? 1 : 2) extends + (() => T extends Y ? 1 : 2) ? true : false // JSX support type IsAny = 0 extends (1 & T) ? Y : N // type Convert = { [Property in keyof T as `on${KebabToPascal}` ]: T[Property] extends IsAny ? any : (e: CustomEvent) => void } type KebabToCamel = T extends `${infer H}-${infer J}${infer K}` -? `${Uncapitalize}${Capitalize}${KebabToCamel}` -: T; + ? `${Uncapitalize}${Capitalize}${KebabToCamel}` + : T; type KebabToPascal = Capitalize>; type GlobalHTMLAttributeNames = "accesskey" | "autocapitalize" | "autofocus" | "autocomplete" | "contenteditable" | "contextmenu" | "class" | "dir" | "draggable" | "enterkeyhint" | "hidden" | "id" | "inputmode" | "lang" | "nonce" | "part" | "exportparts" | "pattern" | "slot" | "spellcheck" | "style" | "tabIndex" | "tabindex" | "title" | "translate" | "ref" | "inert"; type ElementProps = Partial>; -type Convert = { [Property in keyof T as `on${KebabToPascal}` ]: IsAny) => void> } +type Convert = { [Property in keyof T as `on${KebabToPascal}`]: IsAny) => void> } /** * @class @@ -332,25 +332,25 @@ abstract class UI5Element extends HTMLElement { * Called every time before the component renders. * @public */ - onBeforeRendering(): void {} + onBeforeRendering(): void { } /** * Called every time after the component renders. * @public */ - onAfterRendering(): void {} + onAfterRendering(): void { } /** * Called on connectedCallback - added to the DOM. * @public */ - onEnterDOM(): void {} + onEnterDOM(): void { } /** * Called on disconnectedCallback - removed from the DOM. * @public */ - onExitDOM(): void {} + onExitDOM(): void { } /** * @private @@ -371,7 +371,29 @@ abstract class UI5Element extends HTMLElement { characterData: canSlotText, attributeFilter: ["slot"], }; - observeDOMNode(this, this._processChildren.bind(this) as MutationCallback, mutationObserverOptions); + observeDOMNode(this, this.onObservedMutation.bind(this) as MutationCallback, mutationObserverOptions); + } + + onObservedMutation(changes: MutationRecord[]) { + let shouldProccessChildren = false; + + changes.forEach(change => { + // we observe all changes of the component except its slot attribute + if (change.target === this && change.type !== "attributes") { + shouldProccessChildren = true; + } + + const directChildren = [...this.childNodes].includes(change.target as ChildNode); + + // we observe slot attribute change only on the direct child of the web component + if (directChildren && change.type === "attributes") { + shouldProccessChildren = true; + } + }); + + if (shouldProccessChildren) { + this._processChildren(); + } } /** @@ -413,7 +435,7 @@ abstract class UI5Element extends HTMLElement { } const autoIncrementMap = new Map(); - const slottedChildrenMap = new Map>(); + const slottedChildrenMap = new Map>(); const allChildrenUpgraded = domChildren.map(async (child, idx) => { // Determine the type of the child (mainly by the slot attribute) @@ -769,7 +791,7 @@ abstract class UI5Element extends HTMLElement { * * @public */ - onInvalidation(changeInfo: ChangeInfo): void {} // eslint-disable-line + onInvalidation(changeInfo: ChangeInfo): void { } // eslint-disable-line updateAttributes() { const ctor = this.constructor as typeof UI5Element; @@ -798,7 +820,7 @@ abstract class UI5Element extends HTMLElement { // suppress invalidation to prevent state changes scheduling another rendering this._suppressInvalidation = true; - try { + try { this.onBeforeRendering(); if (!this._rendered) { From f153ed9dccc115a74f40cd9198930e8460bbc251 Mon Sep 17 00:00:00 2001 From: Nayden Naydenov Date: Wed, 26 Feb 2025 10:09:23 +0200 Subject: [PATCH 3/4] chore: fix --- packages/base/src/UI5Element.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/base/src/UI5Element.ts b/packages/base/src/UI5Element.ts index 86163b6f855c..1b61ff446e2a 100644 --- a/packages/base/src/UI5Element.ts +++ b/packages/base/src/UI5Element.ts @@ -367,11 +367,11 @@ abstract class UI5Element extends HTMLElement { const canSlotText = metadata.canSlotText(); const mutationObserverOptions: MutationObserverInit = { childList: true, - subtree: canSlotText, + subtree: true, characterData: canSlotText, attributeFilter: ["slot"], }; - observeDOMNode(this, this.onObservedMutation.bind(this) as MutationCallback, mutationObserverOptions); + observeDOMNode(this, this._processChildren.bind(this) as MutationCallback, mutationObserverOptions); } onObservedMutation(changes: MutationRecord[]) { From 2255e13142fe9bdc5208384d3369508683ead123 Mon Sep 17 00:00:00 2001 From: Nayden Naydenov Date: Wed, 26 Feb 2025 11:11:22 +0200 Subject: [PATCH 4/4] chore: fix build --- packages/base/src/UI5Element.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/base/src/UI5Element.ts b/packages/base/src/UI5Element.ts index 1b61ff446e2a..a89b3a642357 100644 --- a/packages/base/src/UI5Element.ts +++ b/packages/base/src/UI5Element.ts @@ -369,12 +369,11 @@ abstract class UI5Element extends HTMLElement { childList: true, subtree: true, characterData: canSlotText, - attributeFilter: ["slot"], }; - observeDOMNode(this, this._processChildren.bind(this) as MutationCallback, mutationObserverOptions); + observeDOMNode(this, this.handleMutationChange.bind(this) as MutationCallback, mutationObserverOptions); } - onObservedMutation(changes: MutationRecord[]) { + handleMutationChange(changes: MutationRecord[]) { let shouldProccessChildren = false; changes.forEach(change => { @@ -386,7 +385,7 @@ abstract class UI5Element extends HTMLElement { const directChildren = [...this.childNodes].includes(change.target as ChildNode); // we observe slot attribute change only on the direct child of the web component - if (directChildren && change.type === "attributes") { + if (directChildren && change.type === "attributes" && change.attributeName === "slot") { shouldProccessChildren = true; } });