Skip to content

Commit

Permalink
fix(framework): Custom setter works when defined in base class and do…
Browse files Browse the repository at this point in the history
…es not include HTMLElement props (#8793)

downport #8718 and #8643
  • Loading branch information
pskelin committed Apr 18, 2024
1 parent e161fa5 commit 9eb7325
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 23 deletions.
19 changes: 18 additions & 1 deletion packages/base/src/UI5Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,23 @@ function _invalidate(this: UI5Element, changeInfo: ChangeInfo) {
this._invalidationEventProvider.fireEvent("invalidate", { ...changeInfo, target: this });
}

/**
* looks up a property descsriptor including in the prototype chain
* @param proto the starting prototype
* @param name the property to look for
* @returns the property descriptor if found directly or in the prototype chaing, undefined if not found
*/
function getPropertyDescriptor(proto: any, name: PropertyKey): PropertyDescriptor | undefined {
do {
const descriptor = Object.getOwnPropertyDescriptor(proto, name);
if (descriptor) {
return descriptor;
}
// go up the prototype chain
proto = Object.getPrototypeOf(proto);
} while (proto && proto !== HTMLElement.prototype);
}

/**
* @class
* Base class for all UI5 Web Components
Expand Down Expand Up @@ -989,7 +1006,7 @@ abstract class UI5Element extends HTMLElement {
throw new Error(`Cannot set a default value for property "${prop}". All multiple properties are empty arrays by default.`);
}

const descriptor = Object.getOwnPropertyDescriptor(proto, prop);
const descriptor = getPropertyDescriptor(proto, prop);
// if the decorator is on a setter, proxy the new setter to it
let origSet: (v: any) => void;
if (descriptor?.set) {
Expand Down
29 changes: 7 additions & 22 deletions packages/base/test/elements/Accessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,27 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
return c > 3 && r && Object.defineProperty(target, key, r), r;
};
import UI5Element from "../../dist/UI5Element.js";
import AccessorBase from "./AccessorBase.js";
import customElement from "../../dist/decorators/customElement.js";
import property from "../../dist/decorators/property.js";
import litRender, { html } from "../../dist/renderer/LitRenderer.js";
let Accessor = class Accessor extends UI5Element {
let Accessor = class Accessor extends AccessorBase {
constructor() {
super(...arguments);
this.storage = false;
}
set myProp(value) {
this.storage = value;
}
get myProp() {
return this.storage;
}
render() {
return html `<div>${this.myProp}</div>`;
}
};
__decorate([
property({type: Boolean})
], Accessor.prototype, "myProp", null);
Accessor = __decorate([
customElement({
tag: "ui5-test-accessor",
renderer: litRender,
})
], Accessor);
__decorate([
property({ type: String, defaultValue: undefined })
], Accessor.prototype, "title", void 0);
Accessor.define();
export default Accessor;

Expand All @@ -46,16 +39,8 @@ import litRender, { html } from "@ui5/webcomponents-base/dist/renderer/LitRender
renderer: litRender,
})
class Accessor extends UI5Element {
storage: boolean = false;
@property()
set myProp(value: boolean) {
this.storage = value;
}
get myProp() {
return this.storage;
}
@property({ type: String, defaultValue: undefined})
title!: string;
render() {
return html`<div>${this.myProp}</div>`;
Expand Down
61 changes: 61 additions & 0 deletions packages/base/test/elements/AccessorBase.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
return c > 3 && r && Object.defineProperty(target, key, r), r;
};
import UI5Element from "../../dist/UI5Element.js";
import customElement from "../../dist/decorators/customElement.js";
import property from "../../dist/decorators/property.js";
let AccessorBase = class AccessorBase extends UI5Element {
constructor() {
super(...arguments);
this.storage = false;
}
set myProp(value) {
this.storage = value;
}
get myProp() {
return this.storage;
}
};
__decorate([
property({type: Boolean})
], AccessorBase.prototype, "myProp", null);
AccessorBase = __decorate([
customElement({
})
], AccessorBase);
export default AccessorBase;

/*
import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
import property from "@ui5/webcomponents-base/dist/decorators/property.js";
import litRender, { html } from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
@customElement({
tag: "ui5-test-accessor",
renderer: litRender,
})
class Accessor extends UI5Element {
storage: boolean = false;
@property()
set myProp(value: boolean) {
this.storage = value;
}
get myProp() {
return this.storage;
}
render() {
return html`<div>${this.myProp}</div>`;
}
}
Accessor.define();
export default Accessor;
*/
12 changes: 12 additions & 0 deletions packages/base/test/specs/Accessor.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,16 @@ describe("Framework boot", async () => {
assert.strictEqual(res2[2], false, "internal storage is updated");
});

it("should stop searching for accessors when HTMLElement is reached", async () => {
const res = await browser.executeAsync( async (done) => {
const el = document.getElementById("accessor");
await window["sap-ui-webcomponents-bundle"].renderFinished();
// return as string, seems that WDIO changes return undefined to null
done([`${el.title}`]);
});

// HTMLElement.title will always return '', but if the getter is coming from the framework, it should return undefined
assert.strictEqual(res[0], "undefined", "getter is coming from field, not HTMLElement.(get title)");
});

});

0 comments on commit 9eb7325

Please sign in to comment.