Downlevel static properties initializer to be contained within the class IIFE to aid Uglify in dead code removal

TypeScript currently down-levels classes with static property initializers being outside of the class IIFE when targeting ES5. This prevents Uglify 2.8+ from removing the class from the final application bundle (after the class IIFEs have been independently annotated with the /*#__PURE__*/ annotation (see #13721) and there are no other references to the class).

Changing the down-leveling to keep the assignment within the IIFE just as static methods are down-leveled would resolve this problem and make lots of code eligible for dead code elimination by uglify (after the IIFEs have been marked with the pure annotation).

The change should be backwards compatible unless the current down-leveling approach was intentional to deal with some unusual corner-case that I'm not aware of.

TypeScript Version: 2.2.1 / nightly (2.2.0-dev.201xxxxx)

Code

class MyHappyClass {
  static myStaticProp = 'this is a simple static prop';
  static myStaticMethod() { return 'static method' };
}

Expected behavior:

var MyHappyClass = (function () {
    function MyHappyClass() {
    }
    MyHappyClass.myStaticMethod = function () { return 'static method'; };
    MyHappyClass.myStaticProp = 'this is a simple static prop';
    return MyHappyClass;
}());

Actual behavior:

The code above downlevels to the static prop being initialized

var MyHappyClass = (function () {
    function MyHappyClass() {
    }
    MyHappyClass.myStaticMethod = function () { return 'static method'; };
    ;
    return MyHappyClass;
}());
MyHappyClass.myStaticProp = 'this is a simple static prop';

Playground link

Just to clarify, this is needed in conjunction with #13721. this change without #13721 is not sufficient, correct?

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) not quite. I already have a tool that adds the pure annotations as requested in #13721. it's a bit hacky, but works. Correcting the static prop downleveling is much harder to do as a transform on the ES5 code, so I'd prefer it to be implemented in tsc rather than outside.

(just to clarify, even though I have a tool for #13721, I still think that having typescript emit this annotation is quite critical for all uglify users, which is most of the client-side web today, because one shouldn't need to resort to hacks in order to have code downleveled by typescript to be removed if not needed).

The emitted goes into phases, the first is to emit an ES6 class from a TS class, and then ES6 class to ES5 class. that is why you see the class written out in such way. doing it all in one closure is doable but will require some wrangling on our side and some hacks here and there to get it to work. not a simple change though.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) static props are quite common and large parts of Angular and Angular Material are currently retained because of this issue, so fixing it along with #13721 would have a huge impact on all client-side code transpiled by typescript.

This related Babel issue outlines a pure annotation plugin for down-levelled ES6 classes and a proof-of-concept hack for the static method IIFE problem.

Igor Minar (@IgorMinar)

Igor Minar (@IgorMinar), would not this be considered an uglify bug? you did tell uglify that this value should is sage to be removed using /*#__PURE__*/ , but yet it keeps it around just because of a write to it.

you did tell uglify that this value should is sage to be removed using /*#__PURE__*/, but yet it keeps it around just because of a write to it.

It's a write to a property, so it's trickier. Uglify does not perform SSA. Pull requests are welcome.

We're going for low hanging fruit here. It is much easier for the code generator to emit the static properties within an IIFE marked with a pure annotation comment out front.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) I agree with kzc, the effort on emitting this correctly is much lower than doing full static analysis, which you have done with info at much higher fidelity.

Uglify would also need to be sure that the assignment is not into a setter which could have non-local side-effect, and right there things get very complicated very quickly.

I don't know the Typescript code base but the proof-of-concept Babel hack to move static properties into the IIFE was a few lines.

Will this also be an issue if you try to use Uglify on ES6 output? For example:

// es6 output
class MyHappyClass {
  static myStaticMethod() { return 'static method'; };
}
MyHappyClass.myStaticProp = 'this is a simple static prop';

Ron Buckton (@rbuckton)

Ron Buckton (@rbuckton) I see what you're getting at - there's no way to set a static property within an ES2015 class definition. uglify-es does not drop the unused class if it has static property assignments.

This annotation proposal is for ES5 code generation only. Typescript ES6 code generation can remain the same.

The previously mentioned Babel hack relies on the static property proposal implemented in babel-plugin-transform-class-properties, which if I'm not mistaken is also valid typescript:

class C { 
    method() {}
    static static_method() {} 
    static static_prop = 1; 
}

--> babel + babel-plugin-transform-class-properties + hack -->

// ...polyfills...

var C = /*#__PURE__*/function () {
  function C() {
    _classCallCheck(this, C);
  }

  _createClass(C, [{
    key: "method",
    value: function method() {}
  }], [{
    key: "static_method",
    value: function static_method() {}
  }]);

  C.static_prop = 1;
  return C;
}();

Ron Buckton (@rbuckton)

Interesting turn of events. So Ron Buckton (@rbuckton) is basically pointing out that if we uglify were to operate on ES2015 code rather than on ES5 then there would be no way for uglify to remove the unused class with a static property, because it doesn't understand that the static prop initialization has no non-local side-effects.

It sounds like this is a problem that will need to be dealt with in uglify for es2015, but today, most of the people downlevel code to es5. So for the es2015->es5 transform, this issue is still relevant I believe.

Typescript could also solve the ES2015 "unused class with static properties and decorators" problem by emitting a /*@__PURE__*/ IIFE wrapper.

Consider this typescript example:

$ cat hello.ts
function sealed(constructor: Function) {
    Object.seal(constructor);
    Object.seal(constructor.prototype);
}

@sealed
class UnusedClass {
    static static_prop = "static prop";
    static static_method() {}
}

console.log("hello");

Generate ES2015 code:

$ node_modules/.bin/tsc hello.ts --target es6 --experimentalDecorators

$ cat hello.js
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
function sealed(constructor) {
    Object.seal(constructor);
    Object.seal(constructor.prototype);
}
let UnusedClass = class UnusedClass {
    static static_method() { }
};
UnusedClass.static_prop = "static prop";
UnusedClass = __decorate([
    sealed
], UnusedClass);
console.log("hello");

The code above would retain the unused class with uglify-es.

However, if typescript were to generate the following for ES2015:

$ cat hello-pure.js
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
function sealed(constructor) {
    Object.seal(constructor);
    Object.seal(constructor.prototype);
}
let UnusedClass = /*@__PURE__*/function(){
    let UnusedClass = class UnusedClass {
        static static_method() { }
    };
    UnusedClass.static_prop = "static prop";
    UnusedClass = __decorate([
        sealed
    ], UnusedClass);
    return UnusedClass;
}();
console.log("hello");

Then uglify-es could drop the unused class:

$ node_modules/uglify-es/bin/uglifyjs hello-pure.js -c toplevel,passes=3
this&&this.__decorate,console.log("hello");

The ES2015 IIFE would only be necessary if there is at least one class decorator or static property for a given class.

Igor Minar (@IgorMinar)

Igor Minar (@IgorMinar) to what extent do you expect this to operate correctly? To get the behavior in #13721, you'd need to ensure that the static initializers themselves are side-effect free, which you'd only be able to guarantee for a limited set of simple expressions (e.g. literal expressions).

Is there a common example of static literal initializers that you've found? Is it okay if other expressions can't be guaranteed as side-effect free?

for changing how the downleveling of the static props works there is nothing risky about the change as far as I can tell.

the bigger question is about whether #13721 and I'll answer that part on that issue.

What about class decorators? Should they be in the IIFE as well?

class decorators can not be guaranteed to be side effect free. so even if we emit them in the same closure, the uglify flag will not be emitted.

Just for the sake of argument, let's say a class decorator has a side effect. Can you provide an example where a typescript user would want that decorator to be called and the code for that class retained for an otherwise unused class? I'm not trying to be difficult, I'm just not aware of a real-life use case.

if the decorator registers a class in a global registry (e.g. MEF like registration for dynamic instantiation). that would be the only use (or call) to the class. eliding the class is exactelly the opposite of what you want.

kzc

kzc Angular uses decorators only as a carrier for metadata during design-time or dev mode. In prod mode, we can safely delete them all and we have a tool to do that that will be rolled out via angular-cli soon. So in short, decorators, whether class or property decorators are not a concern for us because we strip completely in production bundles.

eliding the class is exactelly the opposite of what you want.

On the contrary! That would be exactly what I'd want as a typescript user. :-)

If the outermost decorator is a special decorator - @elide or whatever - then the typescript user could be in charge of that decision to ignore side effects for specific classes - be they static properties or other decorators.

On the contrary! That would be exactly what I'd want as a typescript user. :-)

not sure i understand your comment..

@register("MyExtension")
class C {}

you would assume that this is an extension, and a framework at runtime would instantiate the class, and use it. eliding it would not help.

I'm saying the programmer should have the option to decide on a class by class basis whether to ignore side effects for static properties and decorators.

But now that I think about it, it could be done without a language extension if Typescript were to merely preserve comments in the following example:

function deco(constructor: Function) {
    Object.seal(constructor);
    Object.seal(constructor.prototype);
}

let Greeter = /*@__PURE__*/function(){
    @deco
    class Greeter {
        static se = side_effect();
        greeting: string;
        constructor(message: string) {
            this.greeting = message;
        }
        greet() {
            return "Hello, " + this.greeting;
        }
    }
    return Greeter;
}();

It presently drops the comment in the emitted code:

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
function deco(constructor) {
    Object.seal(constructor);
    Object.seal(constructor.prototype);
}
var Greeter = function () {
    var Greeter = (function () {
        function Greeter(message) {
            this.greeting = message;
        }
        Greeter.prototype.greet = function () {
            return "Hello, " + this.greeting;
        };
        return Greeter;
    }());
    Greeter.se = side_effect();
    Greeter = __decorate([
        deco
    ], Greeter);
    return Greeter;
}();

I'm saying the programmer should have the option to decide on a class by class basis whether to ignore side effects for static properties and decorators.

that is a different feature request then. generally we have not had type-directed or decorator-directed emit. it has been one of the design goals (see # 5)

But now that I think about it, it could be done without a language extension if Typescript were to merely preserve comments in the following example:

persevering comments is doable. but i doubt anyone wants to write code that looks like this. they usually want their tools to do that kind of work.

persevering comments is doable.

Preserving comments before function calls would be useful. There would be no alternative to drop such a class otherwise.

Navigation Menu

Repository navigation

Downlevel static properties initializer to be contained within the class IIFE to aid Uglify in dead code removal #15857

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Downlevel static properties initializer to be contained within the class IIFE to aid Uglify in dead code removal #15857

Description

Activity

mhegazy commented on May 15, 2017

IgorMinar commented on May 15, 2017

mhegazy commented on May 15, 2017

IgorMinar commented on May 15, 2017

kzc commented on May 16, 2017

mhegazy commented on May 16, 2017

kzc commented on May 17, 2017

IgorMinar commented on May 17, 2017

kzc commented on May 17, 2017

rbuckton commented on May 17, 2017

kzc commented on May 17, 2017

IgorMinar commented on May 17, 2017

kzc commented on May 18, 2017

DanielRosenwasser commented on May 18, 2017

IgorMinar commented on May 18, 2017

kzc commented on May 18, 2017

mhegazy commented on May 18, 2017

kzc commented on May 18, 2017

mhegazy commented on May 18, 2017

IgorMinar commented on May 18, 2017

kzc commented on May 18, 2017

mhegazy commented on May 18, 2017

kzc commented on May 18, 2017

mhegazy commented on May 18, 2017

kzc commented on May 18, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions