Getters/Setters with Object.defineProperty

[artfiedler]

Bug report

Input code

// This will be an internal class determined by entry point
export class testProperties {
    private _salad: number = 0;
    public dressing: number = 0;
    public get salad(): number { return this._salad; }
    public set salad(val: number) { this._salad = val; }
}

var test = new testProperties();
test.salad = 0;
test.dressing = 0;
var totalSalad = test.salad + 1;
var totalDressing = test.dressing + 0;

Expected output

(function(exports){
exports.testProperties = void 0;
    var testProperties = (function () {
        function testProperties() {
            this._private__salad = 0;
            this._internal_dressing = 0;
        }
        Object.defineProperty(testProperties.prototype, "_internal_salad", {
            get: function () { return this._private__salad; },
            set: function (val) { this._private__salad = val; },
            enumerable: false,
            configurable: true
        });
        return testProperties;
    }());
    exports.testProperties = testProperties;
    var test = new testProperties();
    test._internal_salad = 0;
    test._internal_dressing = 0;
    var totalSalad = test._internal_salad + 1;
    var totalDressing = test._internal_dressing + 0;
})(output);

Actual output

(function(exports){
exports.testProperties = void 0;
    var testProperties = (function () {
        function testProperties() {
            this._private__salad = 0;
            this._internal_dressing = 0;
        }
        Object.defineProperty(testProperties.prototype, "salad", {
            get: function () { return this._private__salad; },
            set: function (val) { this._private__salad = val; },
            enumerable: false,
            configurable: true
        });
        return testProperties;
    }());
    exports.testProperties = testProperties;
    var test = new testProperties();
    test._internal_salad = 0;
    test._internal_dressing = 0;
    var totalSalad = test._internal_salad + 1;
    var totalDressing = test._internal_dressing + 0;
})(output);

Additional context The "salad" argument of Object.defineProperty was not renamed to "_internal_salad"

[timocov]

@artfiedler can you please check what exactly is used in exports from testProperties class in your case? I see that the tool doesn't rename salad in Object.defineProperty(testProperties.prototype, "salad", {, but other issues it seems that come from you export something like that from your entry point: export const foo = testPropertiesInstance.salad + 1, is it right? It'd be awesome if you'll adopt your code to avoid your specifics.

I'll try to fix the issue with defining property and with detecting whether a property is exported property (currently the tool cover cases like I mentioned above incorrectly), but I'm not sure whether it'll fix your cases.

So, right now I see the following issue:

input.ts

class TestProperties {
    private _salad: number = 0;
    public dressing: number = 0;
    public get salad(): number { return this._salad; }
    public set salad(val: number) { this._salad = val; }
}

const test1 = new TestProperties();
test1.salad = 0;
test1.dressing = 0;
export const totalSalad = test1.salad + 1; // the problem is here
export const totalDressing = test1.dressing + 0; // and here - the tool treats accessing to test1.dressing here as dressing in "public"

output.js

Object.defineProperty(exports, "__esModule", { value: true });
var TestProperties = /** @class */ (function () {
    function TestProperties() {
        this._private__salad = 0;
-        this.dressing = 0;
+        this._internal_dressing = 0;
    }
-    Object.defineProperty(TestProperties.prototype, "salad", {
+    Object.defineProperty(TestProperties.prototype, "_internal_salad", {
        get: function () { return this._private__salad; },
        set: function (val) { this._private__salad = val; },
        enumerable: true,
        configurable: true
    });
    return TestProperties;
}());
var test1 = new TestProperties();
-test1.salad = 0;
-test1.dressing = 0;
-exports.totalSalad = test1.salad + 1;
-exports.totalDressing = test1.dressing + 0;
+test1._internal_salad = 0;
+test1._internal_dressing = 0;
+exports.totalSalad = test1._internal_salad + 1;
+exports.totalDressing = test1._internal_dressing + 0;

[artfiedler]

Honestly, in my case, I just created a dummy testProperties.ts inside my existing larger project, with that typescript code mentioned above, my project has no references to it or includes etc in my project to that class. I'm new to your transform and I'm testing potential problems I might run into and seeing what is renaming and not renaming etc.

In general I hate adopting specific coding patterns to work around a potential issue because I'll have existing code already that I dont remember it all, and then a year from now maybe I wont remember why I coded it that way LOL. I try to keep everything pretty plain.

[timocov]

I'll publish the new version with fixes for this soon, so you can test it and say whether it fixes your case or not.

[timocov]

@artfiedler just published v0.4.0. Can you please install and check it?

[artfiedler]

I just stepped away, won't be back on until tonight. Thanks for the fast turn around!

Sent from my Windows 10 device

From: Evgeniy Timokhov Sent: Monday, June 29, 2020 9:34 AM To: timocov/ts-transformer-properties-rename Cc: Arthur Fiedler; Mention Subject: Re: [timocov/ts-transformer-properties-rename] Getters/Setters withObject.defineProperty (#7)

@artfiedler just published v0.4.0. Can you please install and check it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[artfiedler]

Reading over both outputs from transpile and terser mangle it looks like it works perfect. Also you may want to get the link on ttypescript project updated to properties-rename transform, I saw your old repo for the minifier listed there and decided to check what other repo's you had and found this one :)

[timocov]

I already created PR for that https://github.com/cevek/ttypescript/pull/97, but it isn't merged yet :)