microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.81k stars 12.35k forks source link

es2022 transpiles class fields to static init blocks #48145

Open bennypowers opened 2 years ago

bennypowers commented 2 years ago

When useDefineForClassFields is false, es2022 transforms simple static class fields into class static init blocks. This is a problem for users that need useDefineForClassFields (lit requires this). As well, class fields and class private members are widely supported, where class static init blocks are not. Class fields have smaller bytecounts than transformed output, and are easier to read. I understand that the code is equivalent on an engine which supports static class blocks, but this presents a serious problem for some users, and I think TSC should provide an escape hatch.

Don't get me wrong, I think static blocks are super cool, and I'm looking forward to using them, but many users will want to use es2022 to get native private class members, which are widely supported, but they'll get stuck with static init blocks that they didn't write in source - a syntax error in most toolchains and on apple's monopoly engine webkit

https://www.typescriptlang.org/play?target=9#code/MYGwhgzhAECC0G8BQ1XQgFzBglsaY0AvNAORikDcSAvkA

input:

class A {
  static a = 'a';
}

output:

"use strict";
class A {
    static { this.a = 'a'; }
}

In my case, I have library sources that use decorators and need to be compiled with set semantics, but I also want them to have untranspiled private fields, which are widely supported.

Thank you for considering this case.

Originally posted by @bennypowers in https://github.com/microsoft/TypeScript/issues/46291#issuecomment-1060054758

RyanCavanaugh commented 2 years ago

I don't really like any of the implied solutions here. Not using useful ES2022 features in ES2022 emit raises the question of why we would call that target ES2022. Making a new target, let's call it "ES2021.5", raises the question of what exactly is in that target and how we choose that would be based on a point-in-time evaluation that is difficult to explain in the future.

We've had this situation arise before where the most common runtimes support some subset of the latest-and-greatest spec version but not all of it, and features that were very nice to use un-downleveled couldn't be used with other unsupported latest features. In those cases, the solution was the same as what I'd suggest here - wait it out, or over-downlevel.

Expanding the configuration matrix is a permanent operation, but this is a very transient state of affairs - JavaScriptCore will of course support static blocks at some point. For the set of users that need microtargeting of transforms, it's reasonable to suggest to use a different more-configurable downleveling tool.

trusktr commented 1 year ago

Static class fields are also valid es2022, so there's no need to transpile them to static init blocks if they're already valid es2022 code as written.

Given this valid es2022 input,

class Foo {
  static css = 'foo'
}

TypeScript unnecessarily converts it into this:

class Foo {
    static { this.css = 'foo'; }
}

There isn't any reason for this conversion to happen.

Playground showing unexpected output

^ No decorators are being used in that example.

It might be that TS is pre-emptively using static init blocks for use with decorators. Example with decorators where static init blocks are needed as an implementation detail of decorators:

Playground using decorators

trusktr commented 9 months ago

I don't really like any of the implied solutions here. Not using useful ES2022 features in ES2022 emit raises the question of why we would call that target ES2022. Making a new target, let's call it "ES2021.5", raises the question of what exactly is in that target and how we choose that would be based on a point-in-time evaluation that is difficult to explain in the future.

@RyanCavanaugh I totally hear you on that, but static reactiveProperties = ['colors', 'wingSize'] is es2022 code that need not be converted to anything.

Converting this static foo = 123 to static { this.foo = 123 } is not downleveling.

Is this an accidental side-effect of standard decorator support? If there are no decorators, no conversion needs to happen.

RyanCavanaugh commented 9 months ago

@trusktr it sounds like you're confused by having useDefineForClassFields off

trusktr commented 9 months ago

@trusktr Ah! Sorry. I overlooked that in the current project useDefineForClassFields is set to false, and I forgot that's what the OP was doing. That makes total sense.

As for Lit, I think they need to make their decorator support all permutations in Lit 3 since they still support legacy (@bennypowers if needed I have sample implementations and tests of decorators that support both useDefineForClassFields values, and all 4 permutations of Babel loose/non-loose modes, for a total of 6 possible permutations (I run tests 6 times for my still-legacy decorators), and with standard decorators in TS and Babel this can be expanded to 12 permutations (run all tests 12 times) assuming loose mode still applies for standard decorators (I really hope not, its just a foot gun)).

When Lit moves to version 4 or higher and supports only standard decorators, it can keep useDefineForClassFields enabled so that Lit code is 100% compatible with vanilla JavaScript, and test permutations can be reduced to two (TS and Babel), unless by that time it is out in browsers, then just test the real ones.

I don't think that TypeScript can really do anything about this because the option does exactly what it says it does. Plus as of Safari 16.4 static init blocks are supported, so a new escape hatch option will not be needed for long (yet would need to be supported for a long time).

I think this can be closed.