Skip to content

[Suggestion]: Merge emitted getters and setters into a single Object.defineProperties call #14219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PyroVortex opened this issue Feb 22, 2017 · 6 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@PyroVortex
Copy link

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

Proposal
We could support better minification of class declarations involving ES5 getters and setters by collapsing the calls to Object.defineProperty into a single call to Object.defineProperties, unless there is some performance reason why we currently prefer several calls to Object.defineProperty?

Code
Given the following class

class Foo {
    public get a() {
        return 1;
    }
    public get b() {
        return 2;
    }
}

Current behavior:

var Foo = (function () {
    function Foo() {
    }
    Object.defineProperty(Foo.prototype, "a", {
        get: function () {
            return 1;
        },
        enumerable: true,
        configurable: true
    });
    Object.defineProperty(Foo.prototype, "b", {
        get: function () {
            return 2;
        },
        enumerable: true,
        configurable: true
    });
    return Foo;
}());

Suggested behavior:

var Foo = (function () {
    function Foo() {
    }
    Object.defineProperties(Foo.prototype, {
        a: {
            get: function () {
                return 1;
            },
            enumerable: true,
            configurable: true
        },
        b: {
            get: function () {
                return 2;
            },
            enumerable: true,
            configurable: true
        }
    });
    return Foo;
}());
@mhegazy mhegazy added the Suggestion An idea for TypeScript label Feb 22, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Feb 22, 2017

We would be open to accepting a PR for this.

@jeffreymorlan
Copy link
Contributor

jeffreymorlan commented Feb 22, 2017

This will have to be limited to merging consecutive properties if we want to preserve enumeration order:

class Foo {
	get a() { return 1; }
	normalMethod() {}
	get b() { return 2; }
}
console.log(Object.getOwnPropertyNames(Foo.prototype)); // ['a', 'normalMethod', 'b']

@PyroVortex
Copy link
Author

Would become more worthwhile if combined with the breaking changes that are necessary to bring ES5 emit inline with the ES2015 class specification, namely making both prototype methods and getter/setter properties non-enumerable. At that point it would make sense to have the defineProperties call also handle the prototype methods (since defineProperty and defineProperties are the only ways to add non-enumerable members), which would preserve ordering.

@Rycochet
Copy link

One thing to bear in mind for developers with this is when using UglifyJS with mangle-props afterwards.

(This is informative, so anyone using UglifyJS and that option should be aware of it - and it's pretty easy to work around - the option is default-off, so not going to be relevant to most.)

Using Object.defineProperty is naming the key as a string, so when mangling the property names it will have access mangled but creation not. Changing to Object.defineProperties would mean that both could be mangled the same way, and result in slightly smaller final files as all access to the class member can be reduced to one or two letters.

While I prefer to use descriptive names for all members, in private projects (or public ones that have private members and separate "production" build) it's potentially just wasting a lot of space to have the full text names scattered around everywhere, so merging the declarations like this (and adding all the current "normal" properties) would allow for slightly smaller output files etc.

@mhegazy mhegazy added the Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature label Nov 9, 2017
@Aqours
Copy link

Aqours commented Jan 24, 2018

I need this feature.
TypeScript with this can work with google-closure-compiler in advanced mode.
It's important for JavaScript deep compression.

@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision and removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jun 23, 2021
@RyanCavanaugh
Copy link
Member

Declining based on low interest and increasingly-low relevance. ES5 runtimes are getting pretty rare and the only real upside of this would be a slightly smaller bundle size, which other tools will do a much better job of producing with ES6 output than we ever will with downlevel output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants