Optimization
Minifier
Bundler
IIFE
Wrapper
Terser
TypeScript 3.9 pulled in PR #32011, which tailors TypeScript's output for optimization with minifiers such as Terser. This PR definitely goes in the right direction for optimizing TypeScript's emit for most users, but doesn't necessarily work for all toolchains out there. At Google we have optimization processes after TypeScript compilation that use static analysis to look for classes, do some rough type-checking, and for example rename class properties and drop methods. The wrapper introduced in TypeScript 3.9 hinders our ability to analyze these classes.
We'd like to propose a new flag to toggle this behavior, which I'll call --add-optimization-hints
for now. This flag would control if TypeScript decides to emit these hints for optimizers and bundlers, which is not necessarily limited to the /** @class */
form introduced in TypeScript 3.9. For users such as Google, we can disable this flag and instead rely on our own optimizers to handle this, but users piping directly through Webpack and Terser can still receive the benefits from #32011.
Expanding on our toolchain a bit from above, at Google we use a combination of tsickle, closure-compiler, and other tools to optimize our TypeScript code. Tsickle is used to add type annotations and use the goog.module
module system to TypeScript's emit such that Closure Compiler can optimize the code. Using the TypeScript 3.9 RC which emits IIFE wrappers around ES2015 classes Closure Compiler is unable to understand that this is an alias for the inner class. By setting --add-optimization-hints=false
we would be able to statically analyze TypeScript's output without doing additional special processing.
PR #32011 isn't the first to request optimization hints from the TypeScript output, for example #13721 requested a /** #pure */
annotation as well, and there could be more in the future that TypeScript would like to hint at. Adding this flag allows TypeScript to produce messier code that is less similar to the input, but more compatible with bundlers and minifiers.
This would be a new flag for the TypeScript compiler. Assuming TypeScript 3.9 is being used, given the input:
// input.ts
class C {
static prop = 10;
getProp() {
return C.prop;
}
}
Compiling with tsc --add-optimization-hints=true
:
// output.js
"use strict";
let C = /** @class */ (() => {
class C {
getProp() {
return C.prop;
}
}
C.prop = 10;
return C;
})();
Compiling with tsc --add-optimization-hints=false
:
// output.js
"use strict";
class C {
getProp() {
return C.prop;
}
}
C.prop = 10;
My suggestion meets these guidelines:
Duplicated/related to #8?
It's kind of the inverse. #8 is requesting that TS optimize the code. This bug is requesting that TS mangle the input code less, because it currently does some additional transformation in anticipation of a particular optimizer.
Demo link. This bug is requesting that the output look more like the input, rather than introducing the extra IIFE etc. Ideally there'd be a way to make it nearly verbatim (with types removed, of course).
More specifically, this interferes with Closure Compiler's global optimizations. Terser wants to handle the class as an single opaque value and needs to use the @class annotation to ignore the side effects and the function wrapper to pull them all together.
Closure Compiler doesn't need that, it understands the parts that get pulled together into a class and will properly back off of cases that might have side-effect (such as decorators).
Wrapping the class in a function and returning it means the class is aliased (and escapes).
Is there any chance of this landing in 3.9? This is currently our biggest blocker in the RC.
If you agree with the general idea in the bug, I volunteer @rrdelaney to write the patch. ;)
This proposal would work for Angular β in fact make it easier for us to support our customers using Angular (and TypeScript) at Google as well as outside of Google.
@RyanCavanaugh is this awaiting feedback from us, or from the TS team? :-)
@mprobst from the tooltip on the label:
This means we'd like to hear from more people who would be helped by this feature
Can I understand why #38566 is not the appropriate fix? Why does this need a separate flag?
@DanielRosenwasser yep! So that fix would only apply when targeting ESNext, using define for class fields, and not using decorators. Unfortunately not all of those conditions are met in every configuration. I think the intent of #38566 was to make sure a wrapper isn't added if no downleveling happens and no extra statements are needed after the class declaration. This proposal will allow users outside of that narrow configuration to remove the IIFE without any extra processing.
We (at Lucidchart) have over a million lines of TypeScript code that we could never upgrade to a new version of TS without the fix this flag would provide.
We've decided to revert #32011 and issue a 3.9 patch release. The optimizations in that PR are still valuable and we think it might be appropriate to offer them behind a flag in the future, but wouldn't want that behavior to be the default since it's a less idiomatic emit.
@RyanCavanaugh uh oh.. that's not what we wanted... where we is Angular / Google.
I think that #32011 was the right change for the npm/Terser ecosystem. In fact by reverting it, Angular CLI will have to go back to correcting the previous (TS<3.9.0) emit shape and turning it into the #32011 shape as a post processing step in @angular-devkit/build-optimizer. This needs to happen in every single Angular application outside of Google, and we have to process all application as well as npm library code that is part of the compilation unit which slows down the production builds.
I can see how our requests can seem to be conflicting and confusing.
To be clear for Angular, we need both emit shapes to be supported:
One with #32011, which we'll rely on outside Google where we depend on less sophisticated optimizations performed by Terser which requires hints in the form of /*@__PURE__*/
to remove dead code (the comments need to be in front of IIFEs that contain all the expressions and statements related to a single class - e.g. decorators).
Another without #32011, which we'll rely on at Google (or outside of Google, in apps that use Closure Compiler).
Since terser is the primary minifier used by apps that depend on npm packages, it would be better for the health of the entire web ecosystem if you kept #32011 on by default, and in special cases (Google, Closure Compiler, etc), the alternative emit could be turn on via a compiler flag.
Can you please reconsider the rollback? I do think that by rolling back the change, the web ecosystem that is heavily influenced by TypeScript's emit shape will remain stuck with a lot of code that can't be dead-code-eliminated by Terser (the current most commonly used JS minifier) unless a lot of post-processing happens during the "last mile" build step within all the apps.
Sorry for the bad communication on our side here (we're working to get better at it!).
I just met with @IgorMinar and @rrdelaney and others on our side. We'll write a more coherent answer shortly. No need to rush anything in the meantime! :-)
Thanks for taking a look into this! We really appreciate the thought going into this issue. We (@IgorMinar, @mprobst, @rrdelaney) met, here's our perspective:
We think rolling back for the time being is the right thing to do, and we should reconsider the approach taken in the PR.
A potential solution might be annotating TS emit on a semantic level (e.g. /** @staticproperty */ MyClass.staticProp = 1
), which would allow downstream tools to easily process the output without special casing for a particular optimizer. This also seems to be more in line with TypeScript's design goals, in particular 3. and 4. This will need more research though.
I think this issue represents the wish of the majority of the typescript community. I think it would fit with the ts philosophy, and would optimize the speed and storage of bigger projects
This is already 6 months old, what s the status on this? When can we see this at list with a flag?
I stumbled upon as i was trying to figure out why tree shaking was not working in Nativescript. Nativescript is heavily based on Typescript and decorators. Tree shaking makes a big difference in app startup time. Without that flag/fix we will never be able to improve things further.
EDIT: did anyone did that with a transformer. We might be able to detect static/decorators and wrap in that case? I am not that familiar with transformers but maybe someone found a way.
Most helpful comment
Thanks for taking a look into this! We really appreciate the thought going into this issue. We (@IgorMinar, @mprobst, @rrdelaney) met, here's our perspective:
We think rolling back for the time being is the right thing to do, and we should reconsider the approach taken in the PR.
A potential solution might be annotating TS emit on a semantic level (e.g.
/** @staticproperty */ MyClass.staticProp = 1
), which would allow downstream tools to easily process the output without special casing for a particular optimizer. This also seems to be more in line with TypeScript's design goals, in particular 3. and 4. This will need more research though.