@property decorator incompatible with tsconfig compilerOption useDefineForClassFields

[killerfurbel]

Bug Description

Hello,

I created a custom element and defined a property. However, setting the property did not change the attribute value in DOM.

After debugging a lot, I think I've tracked this down to the typescript compiler. You can just use the simple example from the playground and add "useDefineForClassFields" = true in the compilerOptions.

Also, there is even a warning (but only in DEV mode - would have saved me some time....) in this case:

ui5-webcomponents/packages/base/src/UI5Element.ts

Line 280 in 63de8f0

console.error(`[UI5-FWK] ${(this.constructor as typeof UI5Element).getMetadata().getTag()} has a property [${prop}] that is shadowed by the instance. Updates to this property will not invalidate the component. Possible reason is TS target ES2022 or TS useDefineForClassFields`);

However, as stated in the warning, the solution cannot be to "not use useDefineForClassFields" or "not use ES2022" (which will default useDefineForClassFields to true, see https://www.typescriptlang.org/tsconfig/#useDefineForClassFields)?

Following the documentation, you can easily fix the issue by just adding a declare to the property as this will not emit any JS Code. Since the JS Code (additional defineProperty() after the super-constructor was called, which already generates the getter and setters based on the element metadata):

...

@customElement({
  tag: "my-element",
  renderer: litRender,
})
export class MyElement extends UI5Element {
  @property()
  declare name?: string;

  render() {
    return html `
      <div>
          Hello, ${this.name || "World"}!
      </div>`
  }

...

The only downside is, that you cannot declare a default value for this property then... so if the default for name should be UI5, you would need to write:

...

@customElement({
  tag: "my-element",
  renderer: litRender,
})
export class MyElement extends UI5Element {
  @property()
  declare name?: string;

    constructor() {
        super();
        this.name = "UI5"; // default
    }

  render() {
    return html `
      <div>
          Hello, ${this.name || "World"}!
      </div>`
  }

...

So what's the best option to solve this issue? I think at least the documentation should address this issue and show the alternatives?

Affected Component

No response

Expected Behaviour

No response

Isolated Example

No response

Steps to Reproduce

...

Log Output, Stack Trace or Screenshots

No response

Priority

None

UI5 Web Components Version

2.4.0

Browser

Chrome

Operating System

No response

Additional Context

No response

Organization

No response

Declaration

• I’m not disclosing any internal or sensitive information.

[pskelin]

Thanks for reporting this @killerfurbel , the docs should indeed be improved. The reason for this missing is probably that we expect most packages to be created via the create package script and most people won't really encounter this.
https://sap.github.io/ui5-webcomponents/docs/development/package/

What I will add to the docs regarding this:

• either use ES2021 and lower
• use ES2022 and higher and set useDefineForClassFields: false

I guess the second option is not immediately obvious, but it falls back to your suggestion of declare on the property and initializer in the constructor, but keeps the good syntax. Would that work for you?

[killerfurbel]

Actually, I just followed the docs and somehow got this setting.

As far as I remember, I started with https://sap.github.io/ui5-webcomponents/docs/getting-started/first-steps/#creating-a-project (--> starting with the standard components, not knowing that I would add custom ones later) ... and if you use vite to create the project and select TypeScript instead of JavaScript (--> was't clear to me either, why are the docs choosing JavaScript here, since I need TypeScript later for the decorators, which are not possible in vanilla JavaScript), you end up with these settings: https://github.com/vitejs/vite/blob/main/packages/create-vite/template-vanilla-ts/tsconfig.json

So yes, documentation is something, and your suggestion is good 👍

However, since this is the future direction of typescript, it feels wrong to let people write code which will break somewhere in the future. I think the syntax as proposed in your docs looks better then the alternative, but it doesn't help much, if it's kind of "wrong".
The real issue is, that the decorator adds metadata, which is processed in the super() call in the constructor. However, TypeScript will then process the new property as well, and overwrite the state with a "plain" property, because that is how extending classes works. So the syntax looks nice, but is somehow just not correct.

So the other idea would be to let people write declare and set a default value in the decorator instead?

In the end - your choice! I was just debugging a lot, finally found a solution and wanted to let you know.