fix: ESNext TypeScript compilation option breaks @attr decorator behavior

[rajsite]

🐛 Bug Report

When using a TypeScript configuration of "target": "ESNext" creating elements with fast-foundation and fast-element results in attributes not responding to value updates.

For example, for a button modeled after the fast-button implementation:

class Button extends FoundationButton {

    @attr
    public appearance: ButtonAppearance;

    public connectedCallback(): void {
        super.connectedCallback();
        if (!this.appearance) {
            this.appearance = ButtonAppearance.Outline;
        }
    }
}

If the button is initialized without the appearance attribute set in html, <my-button></my-button>, then we expect the connectedCallback to run and update the appearance property and attribute. Checking buttonInstance.appearance returns the value "outline" as expected but the attribute did not update and the expected style did not render. Checking the adoptedStylesheets shows that the additional stylesheet is not attached indicating the behavior did not run.

Investigating shows that the appearance property is set as an owned values, ie buttonInstance.hasOwnProperty('appearance') === true. That value is shadowing the accessors for the appearance field defined by the decorator.

Stepping through the code it looks like setting ESNext enables native class fields. The initializer for the class field in the above example runs and sets the appearance property with the value undefined during construction (didn't nail down the exact timing). Then future property updates manipulate the owned value instead of triggering the accessors on the prototype chain. Stepping into the appearance assignment in the connectedCallback shows that the accessors are not triggered.

💻 Repro or Code Sample

See expected behavior and current behavior.

🤔 Expected Behavior

When using ESNext, and specifically browser native class fields, I'd expect the same behavior as using "target": "ES2020" in my TypeScript configuration. The following shows a stackblitz webcontainer configured for ES2020 and when running shows a button with red text illustrating the appearance correctly propagated. It also shows the hasOwnProperty flag as returning false: https://stackblitz.com/edit/fast-es2020-bug-button?file=src%2Fmain.ts

😯 Current Behavior

The following stackblitz webcontainer is configured for ESNext and when running shows a button with black text as the appearance property does not receive the update. It also shows the hasOwnProperty flag as returning true: https://stackblitz.com/edit/fast-esnext-bug-button?file=src%2Fmain.ts

💁 Possible Solution

Configure the TypeScript compiler for a target that does not allow ES2022 native browser class fields. For example, in tsconfig.json set "target": "ES2020".

🔦 Context

I would like to leverage native browser features where possible. I may file a separate issue but it would be nice if the other fast builds compiled to a newer TypeScript target than ES6 and relied on consumers who felt like it to use babel, etc. to target older browsers.

Or maybe include both es6 and esnext / es2021 builds in packages so native browser async / await can be used, etc. Benefits are smaller bundle sizes (generator and generator polyfills aren't injected for async / await and other features that are native).

Edit: Looks like the interactions of class fields and accessors is unintuitive at first glance for lots of folks. They newer decorators proposals may have what's needed for attribute interactions.

🌍 Your Environment

• macOS Big Sur 11.5.1 (20G80)
• Version 92.0.4515.107 (Official Build) (x86_64)
• @microsoft/fast-foundation 2.6.2
• @microsoft/fast-foundation 1.4.1
• typescript 4.3.5
• webpack 5.47.1

[astegmaier]

I hit this issue too, and spent a while digging into it before I found this bug report. I thought it would be helpful to share what I found for whoever goes to fix this in the future.

1. You will get the same breaking behavior if you enable the useDefineForClassFields compiler option with any target. This option was introduced in typescript 3.7 to help people gradually move to a the newly-standardized class-semantics from legacy typescript behavior. The release notes have a great explanation about the history.

2. The root cause of the issue is that the @observable and @attr decorators use Reflect.defineProperty to create accessors on the prototype of a class that uses them see code. When the properties are initialized, different things happen depending on how the code is transpiled.

   original typescript:

   class MyClass {
        @observable myProp = "something"
   }
   

   a. tsc default output with target other than ES2022 or ESNext:

   class MyClass {
        constructor() {
            this.myProp = "something"
        }
   }
   

   b. tsc behavior with useDefineForClassFields = true: (note, this similar to how babel and swc work, too)

   class MyClass {
        constructor() {
            Object.defineProperty(this, "myProp", {
               enumerable: true,
               configurable: true,
               writable: true,
               value: "something"
           });
        }
   }
   

   c. tsc output with target ES2022 or ESNext:

   class MyClass {
        myProp = "something"
   }
   

   In cases (b) and (c), the property assignment effectively blocks the getter and setter that were created on the prototype, so future changes to myProp never get seen by the observable system.

3. Because babel and swc output is similar to (b), this is the root cause of why it's impossible to use the @attr and @observable decorators in projects that use those transpilers (see #4503 and this issue I filed with swc).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SamuelQZQ]

This is a really significant bug! Please take action!