search

FredKSchott / create-snowpack-app

The all-in-one app template for Snowpack. [moved]
https://www.snowpack.dev
Other
727 stars 96 forks source link

Enable "useDefineForClassFields" in tsconfig #173

Closed robpalme closed 4 years ago

robpalme commented 4 years ago

This option "useDefineForClassFields": true combined with "target": "esnext" ensures that modern class syntax is emitted.

This option ensures browser and Node semantics are respected. Without this option, TypeScript will down-level class field declarations into constructor assignments.

Example input: class C { field = 1; }

You can test this in the TypeScript playground. https://www.typescriptlang.org/play/index.html?target=99#code/LAKFGMBsEMGdYAQGEEG8EDMCWBTSATBAXgQEYEBfIA

stramel commented 4 years ago

This change seems fine at the surface level to me. Although, I'm unsure I have enough knowledge to speak to this change. @drwpow @FredKSchott any thoughts?

drwpow commented 4 years ago

I actually wasn’t aware of this setting either! Wow that makes a big difference. I know what I’m going to be adding to all my TS projects now! 😄

Though any time you change output it can be risky, I don’t anticipate any problems with this change, because as stated, the end result is just that your output more closely matches your code, which is always a safe bet.

FredKSchott commented 4 years ago

Errrrrrrr it doesn't look like class fields are supported in Node v10, a version that we still need to support. Basing this off of https://node.green/ (public instance class fields)

stramel commented 4 years ago

Hmm should we revert this?

drwpow commented 4 years ago

Does Node need to parse these classes? I thought this was just for web code?

FredKSchott commented 4 years ago

Oh, thanks for the sanity check! I'd thought that this was our master tsconfig for everything, including the CLI. If it's just this one TS template, then it's not broken our CLI.

Larger question then: We don't currently use TypeScript to build our frontend projects. Instead, that template is using our internal esbuild plugin to build, which I don't believe reads from tsconfig. So, I think this was a no-op?