Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect the default setting of the useDefineForClassFields tsconfig option #1354

Closed
kfdf opened this issue Jun 6, 2021 · 2 comments
Closed

Comments

@kfdf
Copy link

kfdf commented Jun 6, 2021

Sometime between 0.11.20 and 0.12.6 esbuild stopped transpiling public class fields into assignments inside the constructor when the target tsconfig setting is set to esnext and useDefineForClassFields is not present. This breaks LitElement decorators that make properties reactive, as described here. It's mentioned there that useDefineForClassFields is set to false by default. Maybe the default value should be respected when transpiling typescript files?

@evanw
Copy link
Owner

evanw commented Jun 6, 2021

The TypeScript language now works like this as of TypeScript 4.3.1, so esbuild has been changed to match. The issue microsoft/TypeScript#34787 has some additional context. I'm not sure why the TypeScript team decided to release this as a silent breaking change without an announcement. But the documentation for LitElement is now incorrect. The default value is false except when the target is "ESNext" in which case the default is true.

Anyway specifying "target": "ESNext" in your tsconfig.json file without also setting a value for "useDefineForClassFields" is now error-prone because it means your code will now behave differently in different versions of TypeScript. I personally think the TypeScript compiler should warn when you do this and tell you to set "useDefineForClassFields" explicitly.

If you are using an old version of TypeScript you can either add "useDefineForClassFields": false in your tsconfig.json file or, if you really don't want to change tsconfig.json, you can also just use an old version of esbuild.

@kfdf
Copy link
Author

kfdf commented Jun 6, 2021

Oh, I see.

@kfdf kfdf closed this as completed Jun 6, 2021
@kfdf kfdf mentioned this issue Jun 6, 2021
daneah added a commit to ithaka/pharos that referenced this issue Dec 28, 2021
As of TypeScript 4.3.1+, [one must specify `useDefineForClassFields:
false`
explicitly](evanw/esbuild#1354 (comment))
when targeting ESNext, because [TC39 now uses the same
syntax](https://www.typescriptlang.org/tsconfig#useDefineForClassFields)
and these would conflict otherwise.

Updating this configuration allows the Storybook configuration to
continue working. Pharos itself never broke because it is currently
targeting `es2019`. Chromatic didn't catch this because it targets
`firefox65` because Chromatic has a really crusty old Firefox. Womp.
daneah added a commit to ithaka/pharos that referenced this issue Dec 29, 2021
* fix(storybook): add useDefineForClassFields false

As of TypeScript 4.3.1+, [one must specify `useDefineForClassFields:
false`
explicitly](evanw/esbuild#1354 (comment))
when targeting ESNext, because [TC39 now uses the same
syntax](https://www.typescriptlang.org/tsconfig#useDefineForClassFields)
and these would conflict otherwise.

Updating this configuration allows the Storybook configuration to
continue working. Pharos itself never broke because it is currently
targeting `es2019`. Chromatic didn't catch this because it targets
`firefox65` because Chromatic has a really crusty old Firefox. Womp.

* fix(pharos): add useDefineForClassFields to main package
sirrah-tam pushed a commit to sirrah-tam/pharos that referenced this issue Dec 1, 2023
* fix(storybook): add useDefineForClassFields false

As of TypeScript 4.3.1+, [one must specify `useDefineForClassFields:
false`
explicitly](evanw/esbuild#1354 (comment))
when targeting ESNext, because [TC39 now uses the same
syntax](https://www.typescriptlang.org/tsconfig#useDefineForClassFields)
and these would conflict otherwise.

Updating this configuration allows the Storybook configuration to
continue working. Pharos itself never broke because it is currently
targeting `es2019`. Chromatic didn't catch this because it targets
`firefox65` because Chromatic has a really crusty old Firefox. Womp.

* fix(pharos): add useDefineForClassFields to main package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants