Explore mangling of private properties in VS Code codebase

[mjbvz]

Background

The compiled JS source of VS Code still contains lots of long names that do not get minified:

Notice how _notebookService and _logSerivce are using their original names

Unlike local variables, it not safe to mangle these names to be shorter for a few key reasons:

• It can break dynamic code. Code such as this['myProp'] would still be using the old property name (myProp) instead of the new mangled name

• It can break public apis. Exported types also have their properties mangled, breaking public apis such as vscode.d.ts

• It can break apis that we consume. For instance, a mangler might rewrite all dom properties. I'm not sure if any tools out there are smart enough to not to do this (some have options to try avoiding mangling dom properties , but I'm not sure any handle all cases of this properly)

However mangling properties is very tempting as it could reduce the bundle size without any significant code changes. Because of above problems however, we need to be smart about how we go about enabling this.

Proposal

I propose that we explore enabling property mangling within the VS Code codebase. Our goals with this:

1. High confidence. Mangling can be dangerous, so we only want to mangle properties if we can be confident that doing so will not cause a runtime break
2. Incremental improvements. Our approach should allow incremental reduction to the bundle size though a series of small steps. This first in with the first goal
3. Make improvements easy, make regressions difficult. It should be easy for devs working on VS code to opt their code into mangling. At the same time, we should make it difficult to regress any improvements that have already been made

Since mangling can be dangerous, for now we will only look into mangling private properties. These theoretically should be safe to transform since they are can only used with a single class. We also don't have too many places where we are looking up private properties dynamically

November

For November, we will start exploring the managing of a very specific subset of private properties: private service properties.

constructor(
	@ICommandService private readonly _commandService: ICommandService,
	@IContextViewService private readonly _contextViewService: IContextViewService,
	@IKeybindingService private readonly _keybindingService: IKeybindingService,
	@ITelemetryService private readonly _telemetryService: ITelemetryService,
) { ... }

This will let us learn more about property mangling and establish tools to use it successful going forward. Even so, I believe a 5% reduction in bundle size is reasonable just from mangling services.

Here's a rough plan for how this work will progress:

1. We will start by only mangling service properties that start with _, such as _someService
2. First to give us more confidence, we will add a lint rule that bans _ from public/protected service properties: eslint: properties starting with underscore are private #165259
3. After fixing an lint errors, we will then enable mangling of just service properties that start with _: Try mangling _private service props #165117
   • Test to make sure debugging the minified build works ok with the new mangled names
4. At this point, if there is team buy-in, we can rename every private service property to start with an _. I have a simple tool that can automate this

Post November

If everything goes well in November, will can next explore mangling more private properties by extending mangling to every property that starts with _.

A rough plan for this:

1. Explore mangling every _ property in some of our built-in extensions. This will let us test this transformation in a smaller codebase before applying it to VS Code
2. If extension maintainers are enthusiastic about this change, give them an eslint rule to require that all private properties start with _. Also provide tools to automate this change
3. If everything looks good, we can extend this to core too. I estimate this could give us another 5% reduction in bundle size without changing any code (only managing properties that already start with _).
4. If we enforce that all properties start with _, than we are more likely in the 10-20% range for bundle size reduction

TODO: Investigate if esbuild can just mangle private props without matching on the name. That would let us avoid requiring the _ . However I'm not sure it is possible. Maybe using plugins?

Long term

In the long term, if we adopt native # private properties mangling should always be safe (indeed # private fields are mangled automatically by most tools). However I don't think we can easily jump right to # privates in core for a few reason:

• Native private properties are only emitted by TS with es2022+, which also brings support for native class fields. With older targets, TS instead use a weak map for private fields. This has a runtime overhead and also could make the bundle larger instead of smaller

  Switching to target es2022 is also not trivial due to useDefineWithClassFields should use-before-init error when class property initializer refers to parameter property TypeScript#50971. At a minimum, we need TS to emit errors so we can catch cases like this

• We use constructor(private prop: Thing) a lot in our code base. TS currently has no plans to support this syntax for native private props.

  Migrating to standard JS syntax is technically correct but results in 3x the amount of TS just to declare and init a property

[jrieken]

In #166126 I have explored a mangler for private and protected properties. It will reduce the final bundle size by ~1.8MB (estimated 15%) by applying a TypeScript to TypeScript transformation. We will have confidence in the output because we can pass the generated TypeScript to the compiler and all down-level checks (tests ect)

[mjbvz]

Yes closing in favor of #166126 which is a safer approach than using esbuild's mangler