-
Notifications
You must be signed in to change notification settings - Fork 34.8k
Description
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:
- 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
- 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
- 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:
- We will start by only mangling service properties that start with
_
, such as_someService
- 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 - 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
- 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:
- 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 - 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 - 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
_
). - 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 smallerSwitching 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