Feedback ♥

[timocov]

If you're using this transformer and like (or don't) it, please share your story (how it helps you, what the size diff you got with it, etc).

[icy0307]

I followed the notorious typescript issue to here.
Awesome work! Looking forward to using it in our application which servers billions of users.
Before diving into it, there are still some concerns which I hope to get your suggestions on.

1. why there is entrySourceFiles option?

Please correct me if I get it wrong: [ts-transformer-properties-rename](https://github.com/timocov/ts-transformer-properties-rename) is a typescript transformer whose role is to perform renaming of private fields, so we won't have to change prefixes by hand in large project. Isn't this renaming process performed on file level? why entry file analysis is needed?
what does this mean?

which uses some approaches from dts-bundle-generator to detect whether some property is accessible via entry points somehow.

2. How safe renames are?
   According to readme, there are still some risks. what are they exactly if I just want to rename all my class private property to private syntax #? bracket notation?

3. How to solve property renaming conflicts?
   module bundler utilizes terser on file level.
   How to pass mangled objects between modules? If we have a base class in chunk A, and derived class in chunk B, how to avoid property mangling between different chunks take webpack terser plugin for example?

[timocov]

Hi @icy0307,

Please correct me if I get it wrong: ts-transformer-properties-rename is a typescript transformer whose role is to perform renaming of private fields, so we won't have to change prefixes by hand in large project. Isn't this renaming process performed on file level? why entry file analysis is needed?

Depends on what you think "private" means, but if you think as "private class fields" then this is not correct. The purpose of this tool is to rename all fields (if possible), not just private and not only from classes. While you can use it to rename private fields only, there is a possibility to rename other fields as well. E.g. lets consider example from readme:

function showMessage(opts: { message: string }): void {
    alert(opts.message);
}
export function alertMessage(message: string): void {
    showMessage({ message });
}

Here you have an exported (i.e. public) from a module function alertMessage and un-exported (i.e. internal) function showMessage. Internal function showMessage takes opts argument object with a property message in it. After transformation this code would like this:

function showMessage(opts) {
    alert(opts._internal_message);
}
function alertMessage(message) {
    showMessage({ _internal_message: message });
}
exports.alertMessage = alertMessage;

As you can see here, opts._internal_message from internal function was renamed despite it is not private in terms of classes but internal in terms of bundle/target build unit. This is very basic example, but shows the power of the tool.

Now let's consider another example where entrySourceFiles option comes and helps. Lets say we have 2 files in our project:

showMessage.ts:

export function showMessage(opts: { message: string }): void {
    alert(opts.message);
}

alertMessage.ts:

import { showMessage } from './showMessage';

export function alertMessage(message: string): void {
    showMessage({ message });
}

The main difference from the code above is that I split functions into separate modules so each function now exported so should be treated as "public" i.e. cannot be just renamed because it is unknown if it is actually exported/public or not. In this case, you can specify entrySourceFiles option e.g. saying entrySourceFiles: ['alertMessage.ts'] that would mean for the tool that only alertMessage.ts file should be interpreted as "public" and everything that is used and exported from it as well, but the rest of the project can be marked as "internal".

And keep in mind this all above is quite basic examples of what actually could be in the project and it should be handled.

what does this mean?
which uses some approaches from dts-bundle-generator to detect whether some property is accessible via entry points somehow.

It is just a note how detecting approach works if you're familiar with that tool. If you are not, you can ignore it. But basically it means "the tool does types usage analysis to perform remaning".

How safe renames are?
According to readme, there are still some risks. what are they exactly if I just want to rename all my class private property to private syntax #? bracket notation?

It's hard to tell for sure, but I don't see any particular reason why it will not work in 100% cases for just private fields. The main concern is "internals" that forces you to follow some guidelines in your project to avoid issues. E.g. if you use any, the tool will not rename a property of this variable, so if you cast or pass it later as other type - you will be screwed up:

interface A {
    value: number;
}
const a: A = {
    value: 42,
};

const b = a as any;
const c = b as A;
console.log(a.value, b.value, c.value);

Theoretically it should print the same value 3 times as all three variables a, b and c refers to the same object. But practically this code would be converted to something like this:

const a: A = {
    _internal_value: 42,
};

const b = a;
const c = b;
console.log(a._internal_value, b.value, c._internal_value);

As you can see here, the tool didn't rename b.value as b is any so no transformation should be applied.

This is one of many possible examples what can go wrong, but what I can say for sure - it is definitely possible and it is not that hard if you think about used types and I've integrated it in https://github.com/tradingview/lightweight-charts where it works so far very well and helped reduce bundle size by more than 10% IIRC.

Note about "think about used types" - the main problem for this tool (and for you if you want to use it on full) in duck typing, which means that an object A of type AType can be assigned and/or used as object of type BType of duck typing allows it (e.g. because there is properties overlapping and overlapped properties have the same type). But the tool can treat these types differently as type AType can be accessible from entry point while type BType not so any usage of AType will not be touched but BType will be.

I'd say that a safer way to integrate this tool is to migrate some small sub-project first if possible and then continue with others. In this case you can control if new code fails as on big projects it could be problematic if you don't have enough e2e tests (not unit tests).

How to solve property renaming conflicts?
module bundler utilizes terser on file level.
How to pass mangled objects between modules? If we have a base class in chunk A, and derived class in chunk B, how to avoid property mangling between different chunks take webpack terser plugin for example?

Not sure that I got this issue tbh. This tool renames all fields in the same way by adding a prefix to them so then you can specify in terser that if a property starts from that prefix then rename it. If you do several terser runs for many files and they don't share mangling cache then I'd suggest to refer to terser documentation in order to find options to pass it over runs.

[icy0307]

Thank you so much for the detailed explanation.
Unfortunately， I work on a massive project with an enormous bundle size, that's why minification is such a pressing matter.
And as a large project, of course, duck typing as a fundamental function for typescript is widely used and cannot be avoided.
ts-transformer-properties-rename seems to be a much more aggressive approach than ts-transformer-minify-privates
However, even only minify private class members helps a lot.

But the critical problem remains, correctness.
If it cannot be done without bugs, it might not well be done at all.

How to make it work with bundlers?
I saw somebody raised the same concerns on your Reddit post, and did a little research

Is there any way to ensure there are no name conflicts between the two classes' private members?
According to terser's documentation, regarding this part, it probably can handle it correctly by providing all files at the same time.

By default, it will mangle all properties in the input code with the exception of built in DOM properties and properties in core JavaScript classes, which is what will break your code if you don't:
Control all the code you're mangling
Avoid using a module bundler, as they usually will call Terser on each file individually, making it impossible to pass mangled objects between modules.
Avoid calling functions like defineProperty or hasOwnProperty, because they refer to object properties using strings and will break your code if you don't know what you are doing.

I know the problem is out of the ts-transformer-minify-privates league, but in order to make it work in large applications in real life, related toolchains must take into consideration.
Is there any way to make it work with bundler? If so, what is the cost despite of building performance? ( parallelization if terser probably would not work, or need more fine-grained control )

[timocov]

How to make it work with bundlers?

Please refer to this section in readme https://github.com/timocov/ts-transformer-properties-rename#how-to-use-the-custom-transformer.

Is there any way to ensure there are no name conflicts between the two classes' private members?

This is a good question, but I think I'm a wrong person to answer. The tool doesn't rename private members into something unreadable, it just prepends a prefix to any of them so overall naming will be the same no matter how many compilation/bundling/building processes you have. The actual minification needs to be handled by other tools like terser so I think this questions is for them.

Is there any way to make it work with bundler?

What do you mean? This tool does a tiny small job (if we considering private members only) and it is to add a prefix to all private members. The rest of job needs to be added by another special for that tools i.e. minificators e.g. terser or esbuild (it has its own internal minifier).

If it cannot be done without bugs, it might not well be done at all.

In javascript any properties renaming is dangerous and doesn't give you 100% correctness. You can use Math.random() to generate a name of the field that you want to access to. Would it break the program at runtime? Yes it would. Does it mean that correctness is not guaranteed? Yes it does. Would you do that? Well, most likely not...

Yes, renaming private fields is the way more stable than renaming all fields and that's I think why most people decide to rename private members only. But either renaming strategy requires you to introduce some limitations into your projects and how you code. In case of renaming any fields these limitations are stricter and there are more of them, and this is the cost for it (with ts and js I believe it's a fair cost).

Duck typing nor casting are not problems for a minification, but their "wrong" usage could be. E.g. if you cast a value from "internal only" type to "internal only" types only, then you most likely will be fine. Or from "public only" to "public only". But the problem is for cross visibility type casting i.e. from public to internal or vice versa. For that reason I said don't rely on duck typing much.

[icy0307]

What do you mean? This tool does a tiny small job (if we considering private members only) and it is to add a prefix to all private members. The rest of job needs to be added by another special for that tools i.e. minificators e.g. terser or esbuild (it has its own internal minifier)

what am I trying to ask is how to make this transformation work with bundlers in the real world.
According to this issue, we can't minify class property without turning off the parallel feature, which significantly prolongs the modification time.
I know it is not this package's scope.
However, would you be so kind to give me some pieces of advice to achieve this without compromising building time? @timocov

[timocov]

we can't minify class property without turning off the parallel feature, which significantly prolongs the modification time.

If you're about private fields, then you can do it with parallelisation as well as you don't need to care if a private property with the same name in different classes would be named differently - it should not be a problem I suppose.