microsoft / TypeScript

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

Private Member Minification Design Proposal #16037

Open mblandfo opened 7 years ago

mblandfo commented 7 years ago

Problem: For front end data binding frameworks such as Angular, Knockout, React, etc, making the choice to use typescript results in larger minified output files.

Proposal: Add a tsconfig option "minifyPrivateMembers" (default false)

Details:

Since front end binding frameworks can have references to members in html, we cannot easily minify these members using existing tooling. For example, using google closure compiler would require using an externs file for all the public properties I want to keep. That would be awful to hand maintain. It'd be really nice if typescript could just minify anything marked private. Typescript wouldn't even have to minify anything else, uglify can do the rest.

Edit: removed incorrect example.

Minification of private members in the greeter example is 18% better than minification in javascript only. I also expect many classes would have many more private members than public members, so savings could be much greater than 18%. Actually with many custom knockout bindingHandlers almost every single member can be made private!

this["greeting"]
var x = "gr" + "eeting";
var y = this[x];

If typescript notices that a class is using string access with variable strings to its own members it could choose to not minify private members for that class. Alternatively if this feature just breaks code like that then I think that's okay as long as it's documented. It will already break external use of private members, and that's why the option defaults to false.

Created from here: https://github.com/Microsoft/TypeScript/issues/8#issuecomment-303423733

emilio-martinez commented 6 years ago

@RyanCavanaugh is this still under consideration?

Since private methods and properties are not used externally, even enforced by the compiler, I think this makes a lot of sense. In the case of use cases like Angular, for example, they enforce public methods and properties with component templates, given they're somewhat of an "external entity" to the class, so I'm use cases like that it shouldn't be a problem.

RyanCavanaugh commented 6 years ago

Regarding the proposed emit, it doesn't work, as described in the FAQ https://github.com/Microsoft/TypeScript/wiki/FAQ#you-should-emit-classes-like-this-so-they-have-real-private-members

We could just rename the identifiers during emit. But this raises more problems -- anyone could derive from the emitted class and declare new members of minified name, breaking both classes at runtime. We'd have to somehow choose identifiers that are both short and unlikely to be used by anyone else, including other TypeScript programs. It's not at all clear that this is possible.

One option is if there were a way for us to signal to a minifier that certain identifiers are "safe" to minify - is this something that those tools support?

mblandfo commented 6 years ago

Ah, good point. It may be possible to create an "externs" file for google closure compiler based on non private members, but that should probably be a plugin or something. Or maybe a standalone program that reads javascript in html.

harquail commented 6 years ago

The latter approach makes a lot more sense for our use case. Currently we will put all public interfaces in separate files and use the AST to find names that are safe to minify based on that. Then, we pass that list of exceptions to UglifyJS. Having a way to emit a list of public member names that are not safe to minify (or the inverse) would greatly simplify our workflow.

This could also work via comment identifiers (similar to /* @class /) that minifiers could use as signals for minification

NoelAbrahams commented 6 years ago

@RyanCavanaugh,

We'd have to somehow choose identifiers that are both short and unlikely to be used by anyone else, including other TypeScript programs. It's not at all clear that this is possible.

Making the prefix configurable would work for most cases, I'd think:

{
  "compilerOptions": {
    "target": "ESx",
   "minify": {
        "prefix": "_"
     }
}

This would generate:

 class  Foo {
    // Privates 
    _a1(){}
    _a2(){}
   ...
 }
Palid commented 6 years ago

Folks, is this PR dead? It'd be lovely to implement this, especially for projects/libraries using dependency injection frameworks like InversifyJS enforcing private variables everywhere. :)

marcusbelcher commented 6 years ago

Ditto for above, I am really in need of minifying private variable names. Is there any timings for this feature to be implemented?

john-lemire commented 6 years ago

@RyanCavanaugh re: "We could just rename the identifiers during emit. But this raises more problems" Is this an intractable problem? Couldn't each class have a private member prefix either in a comment that is also discernable in the .js/d.ts or worst case a private static variable of a known name (ie _) All classes that don't extend anything could start with a value of 'a', all classes that extend from others walk the hierarchy to see what prefix their immediate ancestor used and start at 'b' or whatever the next one is. If the class doesn't extend from anything it's members can just start using a,b,c, etc. for the private member names. If the class extends from something then use the prefix followed by the a, b, c, etc. Private methods may be even easier, ie do private methods really even need to be on the prototype? That's obviously a 4 minute solution that is so simplistic that I'm certain I'm missing many things but this minification thread has been going on for 4 years and we still don't have good solution from the ts team. We really need the ts team to come up with something, I remain unconvinced that a multi party solution is viable. You've highlighted some of the mismatch issues with closure, others have highlighted the problems of others (ie how to keep source maps working when a post process tool starts inlining things). Like everyone else we have been forced to do the duplicate work of incorporating tools into the process, using uglify/webpack, etc. and are years later still getting unsatisfactory results. Please, not too proud to beg, please.

kitsonk commented 6 years ago

If the class doesn't extend from anything it's members can just start using a,b,c, etc. for the private member names. If the class extends from something then use the prefix followed by the a, b, c, etc.

But if then the class is subsequently extended, but other code, consuming this as a library, how do you propose this works to avoid conflicts.

It still seems like this is a problem that TypeScript shouldn't be solving (can't solve). With Uglify2, you can choose to mangle "private" properties based on a RegEx.

For example, --mangle-props regex=/^_/ will only mangle property names that start with an underscore.

Isn't it better to use your mangler/minifier than to impose something upstream?

john-lemire commented 6 years ago

@kitsonk not following you. If I make a class that doesn't extend anything it would have private members a,b,c and someone extends from it their private members would be aa,ab,ac, no conflict. If I make a new version and change the first class to extend something its private members would become aa,ab,ac and the derived class would have to be retranspiled to change its members to ba,bb,bc. They can't use the updated base class without retranspiling the extended class but that's an existing restriction derived classes already have. A non-mangled member name could always be added to a base class that conflicts with a member name already used by a class derived from it in a way that may break functionality

NoelAbrahams commented 6 years ago

@kitsonk,

For example, --mangle-props regex=/^_/ will only mangle property names that start with an underscore.

Isn't it better to use your mangler/minifier than to impose something upstream?

Why would you want to impose the underscore convention on anyone? It's outdated and ugly. That's the reason we have a private keyword.

kitsonk commented 6 years ago

Why would you want to impose the underscore convention on anyone? It's outdated and ugly. That's the reason we have a private keyword.

The sigil of # is coming anyways. (Which gives even more weight to TypeScript imposing something that is non-standard on everyone). You can choose any other format you wish... it is a regular expression. You can impose whatever outdated and ugly syntax you want.

NoelAbrahams commented 6 years ago

So # privates are coming and TypeScript should not minify private methods and we should all use underscore or something equally ugly. I'm having trouble following you.

john-lemire commented 6 years ago

Again there's no need for a regex or any sort of naming convention, the private keyword itself is sufficient. All private method and member names would be minified.

thorn0 commented 6 years ago

@RyanCavanaugh

We could just rename the identifiers during emit. But this raises more problems

So why can't TypeScript simply add a configurable prefix to private members, which would work absolutely great with Uglify's --mangle-props regex=? What kind of problems would this create? Name collisions? Like when a class has a private foo method (compiled to _foo) and its subclass trying to declare _foo explicitly? Isn't this effectively solved by choosing a prefix less likely to get written manually (πρεφιξ2018_foo)? If you're really concerned about uniqueness, generate GUIDs for prefixes.

mblandfo commented 6 years ago

@RyanCavanaugh

We could just rename the identifiers during emit. But this raises more problems -- anyone could derive from the emitted class and declare new members of minified name, breaking both classes at runtime. We'd have to somehow choose identifiers that are both short and unlikely to be used by anyone else, including other TypeScript programs. It's not at all clear that this is possible.

Since .d.ts files list private members, I don't understand this scenario. A derived class in another typescript program that has a name conflict will not compile. I guess it's a bit of work since you have to rename them in the .d.ts output and check the derived class variables inside the same program.

One option is if there were a way for us to signal to a minifier that certain identifiers are "safe" to minify - is this something that those tools support?

I am concerned that this will be a pain in practice, whereas Typescript could solve this and make it easy for users. I suppose @thorn0 makes the point about solving for Uglify.

That also means there's a hack that we could do right now to get uglify to do this: manually prefixing private fields, properties, and functions with "_"

jonnermut commented 6 years ago

I'm wondering if anyone currently has a workable solution for Uglifying only private members/functions in the absence of typescript compiler help that doesn't involve a naming convention for privates?

movedoa commented 6 years ago

Configurable prefix sounds like the best solution and i guess shouldn't be to hard to implement. The only thing needed is something to signal the transpiler do don't do this to specific private members (maybe with an annotation) Any comment on this from the Typescript team?

jonnermut commented 6 years ago

Yes agree a simple configured prefix or template for emitted privates would do the trick.

But I need to get something working soon, so wondering if, until the compiler helps, a solution for private name mangling might be:

  1. Output a d.ts
  2. Remove anything that is private, leaving public & protected
  3. Parse and transform it into the uglify mangle reserved symbols config format. Possibly by regex.

I guess it would be far from optimal, because as I understand it the uglify reserved list is non scoped - just a list of strings (?). So a public symbol in one class would mean the same name as a private symbol would not be mangled.

But probably a lot better than nothing.

It would be nicer to do the inverse (extract and specify the private symbols), but doesn't look like Uglify can do this, and you may end up mangling public things that have the same name.

movedoa commented 6 years ago

Any news on this? Configurable private member prefix would be a huge help for minification

thinwire commented 5 years ago

This is absolutely a problem that the TypeScript team needs to address and not try to shift responsibility entirely to Javascript post-processors. The transformation is best done during compilation, when you have information about the source available, instead of having to guess at it from emitted magic comments or name formatting. Bottom line is, we do not want the private members of a base class to stop the declaration of a field with the same name in a derived class. After all, private members should only be accessed from inside the class itself. One easy way to generate these private names would be to find the depth of the class hierarchy up to that point and prefix the emitted private names with a number of prefix characters equal to depth, e.g.

class A {
    private a: number = 0;
}

class B extends A {
    private a: number = 1;
}

class C extends B {
    private a: number = 2;
}

would result in a generated class C like

function C() {
    this._$a = 0;
    this.__$a = 1;
    this.___$a = 2;
}

This would still leave the private fields human-readable (if only prefixed), non-clashing, source-mappable and targetable from an external tool via regex (matching _+\$ in this case).

Edit: an alternative that is smaller: <prefix><depth><suffix><varname>:

function C() {
    this._1$a = 0;
    this._2$a = 1;
    this._3$a = 2;
}

targetable with _\d+\$

chocolateboy commented 5 years ago

@RyanCavanaugh

We could just rename the identifiers during emit. But this raises more problems -- anyone could derive from the emitted class and declare new members of minified name, breaking both classes at runtime. We'd have to somehow choose identifiers that are both short and unlikely to be used by anyone else, including other TypeScript programs. It's not at all clear that this is possible.

It's not possible and the suggested workarounds are trivial to circumvent.

There is a built-in way to do this safely when targeting ES6: symbols.

before

export default class Example {
    private _foobar = new FooBar();
    private _bazquux = new BazQuux();

    test () { return this._foobar + this._bazquux } // 41 chars minified
}

after

unsafe

export default class Example {
    test () { return this._f + this._b } // 30 chars minified
}

safe

// setup (once per module)
var s = Symbol, f = s(), b = s();

export default class Example {
    test () { return this[f] + this[b] } // 30 chars minified
}

The downside is the overhead of declaring the symbols, which would have to be factored into the calculation of whether each minification is worth it.

Also, by the time this was implemented, emitting #private fields would be an option, which would eliminate the declarations:

ESNext

export default class Example {
    test () { return this.#f + this.#b } // 30 chars minified
}
chocolateboy commented 4 years ago

https://github.com/timocov/ts-transformer-minify-privates (via)