bug: esnext target (Public Instance Fields) breaks State props rerendering

[bashmish]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

2.10.0

Current Behavior

updates of the @State props don't trigger rerendering when tsconfig.json sets target to esnext, because Public Instance Field for @State is shadowing getters/setters defined by StencilJS to observe state changes

Expected Behavior

state updates trigger rerendering

Steps to Reproduce

Just clone https://github.com/bashmish/bug-stenciljs-state-public-field-esnext, npm install && npm start and play with + - buttons.

Or:

1. Create a component with npm init stencil
2. Set compilerOptions.target to esnext in tsconfig.json (activates Public Instance Fields in modern browsers)
3. Add a @State prop and some logic to update it and display in the DOM new value (in my repo I use a simple counter example)
4. Try to update the state and see it doesn't rerender

Code Reproduction URL

https://github.com/bashmish/bug-stenciljs-state-public-field-esnext

Additional Information

Suddenly this problem doesn't happen for @Prop in exactly the same situation.
I debugged why and found this code which remove the shadowing property from the component instance here https://github.com/ionic-team/stencil/blob/16b8ea4dabb22024872a38bc58ba1dcf1c7cc25b/src/runtime/connected-callback.ts#L84
I assume you can use a similar approach for @State and maybe explicitly for @Prop too, because this code purpose seems to be different.

[splitinfinities]

Thank you for the detailed report and the reproduction repo! I flagged this to be validated and @rwaskiewicz will follow up with next steps.

[rwaskiewicz]

Hey @bashmish,

I've confirmed this bug. It appears to be the result of Stencil not handling useDefineForClassFields being enabled by default in TypeScript v4.3 properly. Would adding the following to your tsconfig.json fix the issue temporarily?

{
  ...other fields in tsconfig.json
  useDefineForClassFields: false
}

Either way, I'm going to label this so that it gets ingested into our backlog to get looked at in the near future. Thanks for such a detailed bug report and repro, the team and I really appreciate it! ❤️

[bashmish]

@rwaskiewicz I confirm useDefineForClassFields: false fixes this

good to mention my use case, because I'm limited in what I can do there with tsconfig.json
I work on https://webcomponents.dev and https://backlight.dev which support StencilJS compilation in the browser, where we use the following API to compile StencilJS components:

export declare const transpile: (code: string, opts?: TranspileOptions) => Promise<TranspileResults>;`

so I can't just pass the tsconfig.json and instead can use what TranspileOptions exposes, e.g. for now I had to set target: "es2020" to fix this issue, but I can't set useDefineForClassFields there

ideally I want to be able to just pass the whole tsconfig.json there, not only to workaround this issue, but in general to let users control what they normally can control when using StencilJS via CLI
let me know if it makes more sense to discuss this in a separate feature request, but I wanted to mention it here too because it's closely related

[rwaskiewicz]

Thanks @bashmish, that definitely helps, both from a context standpoint and as a separate feature request. CC'ing the Stencil product manager, @splitinfinities, for visibility

[bashmish]

lit has a similar issue and recommends the same workaround (useDefineForClassFields: false) in https://lit.dev/docs/components/decorators/#decorators-typescript

[rwaskiewicz]

This has been fixed in Stencil v2.19.0, which was just released. I'm going to close this issue as a result. If this bug should reappear, feel free to open a new issue. Thanks!

[bashmish]

Thanks, great job 🤩 I especially like the explainer in the commit message, very clear and detailed, I'm gonna refer to this as an example how to maintain good and healthy history.