google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.4k stars 1.15k forks source link

Optimize / inline getters and setters #1157

Open ochafik opened 9 years ago

ochafik commented 9 years ago

Currently Closure does not inline getters in advanced mode in situations where it inlines methods:

'use strict';
/*
class Foo {
  get bar() { return 123 }
  baz() { return 456 }
}
*/
'use strict';
/** @constructor */
var Foo = function() {};
Object.defineProperties(Foo.prototype, {
  foo: {
    /** @this {Foo} */
    'get' : function() { return 123 }
  }
});
console.log(new Foo().foo)
/** @constructor */
var Bar = function() {};
Bar.prototype.bar = function() { return 456 };
console.log(new Bar().bar())

Gives the following output:

function a() {
}
Object.defineProperties(a.prototype, {a:{get:function() {
  return 123;
}}});
console.log((new a).a);
console.log(456);

It would be nice to add full optimization support for getters/setters.

ochafik commented 9 years ago

Hi @MatrixFrog, actually this is not ES6-specific, the snippet above is ES5 (I just added the ES6 equivalent snippet in the comment to make it easier to read).

DavidSouther commented 7 years ago

Ping? It really sucks to not have get/set foo inlined or minified, and it's been over a year on this issue.

MatrixFrog commented 7 years ago

I don't think this is on anyone's radar currently. Sorry.

DavidSouther commented 7 years ago

Is there a roadmap for ES6 support?

MatrixFrog commented 7 years ago

"Native" ES6 support (meaning, having the option to output ES6 instead of transpiling) is my main project for the next few quarters. So probably within a few months we'll be in a state where getters in ES6 classes are left in their nice shorthand form while Object.defineProperties calls are left in their somewhat longer and less elegant form.

DavidSouther commented 7 years ago

At that point in time, will the following optimization work?

// ES6
class Foo {
  constructor() {
    this.o = {};
  }
  get xx() { return this.o.xx; }
  set xx(v) { this.o.xx = v; }
  static get init() { return 'init'; }
}
let foo = new Foo();
console.log(foo.xx);
// Optimized
console.b((new function(){this.a={}}).a.c)

Currently, the following (equivalent) ES5 gets optimized to the above:

// ES5
/** @constructor */ function Foo() { this.o = {};};
Foo.prototype.get_xx = function() { return this.o.xx; };
Foo.prototype.set_xx = function(v) { this.o.xx = v; };
Foo.get_init = function() { return 'init'; };
let foo = new Foo();
console.log(foo.get_xx());
MatrixFrog commented 7 years ago

I'm not sure, but I think we'd have to add that optimization explicitly (which we can probably do, I just can't promise it would be anytime soon). Historically getters and setters have been uncommon at Google so we haven't devoted time to optimizing them.

ChadKillingsworth commented 7 years ago

They already can be disambiguated/ambiguated/renamed.

Honestly though we'd rather spend time getting ES6 native code working throughout the compiler right now. You can submit PRs to add support to a pass.

theefer commented 7 years ago

I've just encountered this issue too, whereby optimized code doesn't appear to be properly tree-shaken and therefore a lot larger than expected. The source is auto-generated code that includes getters and setters, so unfortunately I can't work around the issue as long as I want to use that generated API.

Lifting optimisation to ES6 code makes sense - is there a place to track progress and timelines on that project?

CrazyPython commented 5 years ago

update?

lauraharker commented 5 years ago

Sorry, no update yet on this. We're trying to finish work on ES6 optimizations this quarter, so that will hopefully free up time to improve getter/setter support.

Created Google internal issue b/120032421

lauraharker commented 5 years ago

Somewhat relevant: right now @jplaisted is fixing a bug with incorrect getter/setter optimizations. (https://github.com/google/closure-compiler/issues/2778).

gkdn commented 5 years ago

FWIW, J2clPropertyInlinerPass does this in a crud way but just for J2CL code.