Add example in README for mangling private class fields

[TimvdLippe]

I was looking into mangling private class fields with Terser.
Even though I think it would be generally safe for Terser
to do this by default, it is currently already possible
by using a regex as --mangle-props.

Therefore, let's add an explicit example in the README
for anybody else who is looking to mangling private
class fields.

[fabiosantoscode]

The caveat "unless you're using quoted properties that start with #" is kind of telling. I think instead of documenting this feature, private property mangling should be moved to regular mangling.

[TimvdLippe]

I am fine with that as well. However, I would prefer if a user can enable private property mangling without mangling any other fields. Since private class fields can safely be mangled and other fields not, clumping them up into 1 flag would prevent users from mangling only private class fields

[fabiosantoscode]

My idea is to use the regular mangler because everyone turns it on (it's super safe) while the property mangler is this advanced feature that the readme tells you you're supposed to be scared of.

[TimvdLippe]

Ah, is the "regular mangler" the one you turn on with compress (which is by default on)? In that case, yes that would work for us.

[fabiosantoscode]

It's enabled with the mangle option, and turns const longVarName into const a. Nobody ever disables it so people forget about it.

It's also really efficient and trivial to implement (besides supporting old safari quirks)

[TimvdLippe]

@fabiosantoscode Is there anything I can do here? Do we want to merge this PR until terser mangles private class fields itself or do you prefer not to?

[jridgewell]

unless you are using quoted properties that start with #

I don't understand this comment. There shouldn't be any conflict between quoted public properties and a private property with the same name.

[TimvdLippe]

At DevTools, we observed the following breakage:

const colorMapping = {
  "#000000": "black"
}

That's because the keys were hexadecimal representations of colors. The fix was easy: refactor to a Map, but we regressed nonetheless. Since terser doesn't differentiate between quoted properties and class fields, you can't do the mapping only for class fields.

[jridgewell]

I'm still a little confused, sorry. Is there an internal CL link that shows the usage? My ldap is jridgewell@.

I think what you're aiming to do is mangle private properties, but you want to keep your object properties? Specifically, some of your object properties use quoted '#000000' hex keys. You've then enabled regex: /^#/ to mangle the private properties, but found it also mangled your hex keys. Is that right?

If so, I see two parts that can be fixed in your setup. First, there's no need to set regex: /^#/ to have Terser mangle your private properties. Just enabling property mangling will turn on private property mangling, regardless of your regex setting. That's because private properties are lexically mangle-able without issue (and I think that's why @fabiosantoscode is suggesting it be on by default when enabling regular mangling). Second, you might need properties: { keep_quoted: true } (or 'strict') to stop Terser from mangling quoted property (but note there's currently a bug which'll be fixed with #1058).

[TimvdLippe]

No, the CL is open-source: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2982146 The regression bug was https://bugs.chromium.org/p/chromium/issues/detail?id=1223298 We enabled private class field mangling in https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2975285

Just enabling property mangling

But we can't do that, as we still have plenty of code that refers to the unmangled name in a separate context. E.g. we can only be certain that we don't break anything with solely mangling with private class fields. So we can't apply the mangling for everything and we need to somehow select only private class fields. The regex solution does exactly that.

I think we could have done the keep_quoted solution, but by now everything is working as expected DevTools-concerned.

[jridgewell]

So we can't apply the mangling for everything and we need to somehow select only private class fields. The regex solution does exactly that.

Private property mangling doesn't use regex when mangling:

terser/lib/propmangle.js

Lines 240 to 246 in d3fb5b0

	if (
	node instanceof AST_ClassPrivateProperty
	|| node instanceof AST_PrivateMethod
	) {
	node.key.name = mangle_private(node.key.name);
	} else if (node instanceof AST_DotHash) {
	node.property = mangle_private(node.property);

You could actually get only private property mangling by setting regex to something that never matches. regex: /$./ will prevent any public properties from being mangled, but because private properties don't care about the regex, they'll still be mangled.

---

To get this back on topic for the PR, I think we can update the description here to demonstrate that private properties will be mangled if --mangle-props is used. Though Fabio might prefer we not merge this, since it should be moved form property mangling into regular mangling.

[TimvdLippe]

Wait, I am getting myself confused. The docs don't mention anything about private property mangling: https://terser.org/docs/cli-usage#cli-mangle-options Does terser already do that?

will prevent any public properties from being mangled, but because private properties don't care about the regex, they'll still be mangled.

Okay, that sounds fine to me. From a user perspective, that is quite weird though. And it isn't mentioned anywhere in the docs. Can we improve terser to do this by default so that we don't have to do this hacky solution with the regex? (I think we are in agreement on this, but the context of private class mangling wasn't clear to me, as I am not familiar with the codebase)

[jridgewell]

Does terser already do that?

Yup!

From a user perspective, that is quite weird though. And it isn't mentioned anywhere in the docs. Can we improve terser to do this by default so that we don't have to do this hacky solution with the regex?

Yes, I think that's the ideal solution.

[fabiosantoscode]

My objective is for this PR to not be necessary, I guess.

As mentioned in the comments above, it's a bit hacky to pass in a regex that never matches, and it also requires knowledge of the internals.

This needs to be done during the regular mangle step, in lib/scope.js, as opposed to in the property mangle step.

[fabiosantoscode]

Fixed in #1060 -- this is now part of regular mangling.

Will release on Monday.

[TimvdLippe]

I can confirm that the new release allowed us to remove our special configuration! https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3158385/1/scripts/build/rollup.config.js Thanks @jridgewell and @fabiosantoscode for fixing the bugs and optimizing more code by default 🎉

[fabiosantoscode]

Awesome stuff!