Skip to content

Commit

Permalink
Fix private getter/setter mangling (#1060)
Browse files Browse the repository at this point in the history 
* Mangle private properties by default when manglin

* Fix sourcemap test for private fields

* Fix property mangling private getter/setter

* tmp

* Improve sourcemap output for private properties

* remove test grep

* Don't print regular string literals
  • Loading branch information
@jridgewell
jridgewell committed Sep 5, 2021
1 parent f04077c commit e6312f4
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 31 deletions.
2 changes: 1 addition & 1 deletion lib/ast.js
View file
Expand Up @@ -1330,7 +1330,7 @@ var AST_ClassProperty = DEFNODE("ClassProperty", "static quote", {
}
}, AST_ObjectProperty);

var AST_ClassPrivateProperty = DEFNODE("ClassProperty", "", {
var AST_ClassPrivateProperty = DEFNODE("ClassPrivateProperty", "", {
$documentation: "A class property for a private property",
}, AST_ClassProperty);

Expand Down
2 changes: 2 additions & 0 deletions lib/minify.js
View file
Expand Up @@ -14,6 +14,7 @@ import { Compressor } from "./compress/index.js";
import { SourceMap } from "./sourcemap.js";
import {
mangle_properties,
mangle_private_properties,
reserve_quoted_keys,
} from "./propmangle.js";

Expand Down Expand Up @@ -204,6 +205,7 @@ async function minify(files, options) {
if (options.mangle) {
toplevel.compute_char_frequency(options.mangle);
toplevel.mangle_names(options.mangle);
toplevel = mangle_private_properties(toplevel, options.mangle);
}
if (timings) timings.properties = Date.now();
if (options.mangle && options.mangle.properties) {
Expand Down
13 changes: 9 additions & 4 deletions lib/output.js
View file
Expand Up @@ -349,9 +349,11 @@ function OutputStream(options) {
var do_add_mapping = mappings ? function() {
mappings.forEach(function(mapping) {
try {
let name = !mapping.name && mapping.token.type == "name" ? mapping.token.value : mapping.name;
if (name instanceof AST_Symbol) {
name = name.name;
let { name, token } = mapping;
if (token.type == "name" || token.type === "privatename") {
name = token.value;
} else if (name instanceof AST_Symbol) {
name = token.type === "string" ? token.value : name.name;
}
options.source_map.add(
mapping.token.file,
Expand Down Expand Up @@ -1824,6 +1826,7 @@ function OutputStream(options) {

if (self.optional) output.print("?");
output.print(".#");
output.add_mapping(self.end);
output.print_name(prop);
});
DEFPRINT(AST_Sub, function(self, output) {
Expand Down Expand Up @@ -2277,8 +2280,10 @@ function OutputStream(options) {
DEFMAP([
AST_ObjectGetter,
AST_ObjectSetter,
AST_PrivateGetter,
AST_PrivateSetter,
], function(output) {
output.add_mapping(this.start, this.key.name);
output.add_mapping(this.key.end, this.key.name);
});

DEFMAP([ AST_ObjectProperty ], function(output) {
Expand Down
60 changes: 41 additions & 19 deletions lib/propmangle.js
View file
Expand Up @@ -59,6 +59,8 @@ import {
AST_ObjectKeyVal,
AST_ObjectProperty,
AST_PrivateMethod,
AST_PrivateGetter,
AST_PrivateSetter,
AST_Sequence,
AST_String,
AST_Sub,
Expand Down Expand Up @@ -140,6 +142,36 @@ function addStrings(node, add) {
}));
}

function mangle_private_properties(ast, options) {
var cprivate = -1;
var private_cache = new Map();
var nth_identifier = options.nth_identifier || base54;

ast = ast.transform(new TreeTransformer(function(node) {
if (
node instanceof AST_ClassPrivateProperty
|| node instanceof AST_PrivateMethod
|| node instanceof AST_PrivateGetter
|| node instanceof AST_PrivateSetter
) {
node.key.name = mangle_private(node.key.name);
} else if (node instanceof AST_DotHash) {
node.property = mangle_private(node.property);
}
}));
return ast;

function mangle_private(name) {
let mangled = private_cache.get(name);
if (!mangled) {
mangled = nth_identifier.get(++cprivate);
private_cache.set(name, mangled);
}

return mangled;
}
}

function mangle_properties(ast, options) {
options = defaults(options, {
builtins: false,
Expand All @@ -161,10 +193,8 @@ function mangle_properties(ast, options) {
if (!options.builtins) find_builtins(reserved);

var cname = -1;
var cprivate = -1;

var cache;
var private_cache = new Map();
if (options.cache) {
cache = options.cache.props;
cache.forEach(function(mangled_name) {
Expand All @@ -187,7 +217,6 @@ function mangle_properties(ast, options) {

var names_to_mangle = new Set();
var unmangleable = new Set();
var private_properties = new Set();

var keep_quoted_strict = options.keep_quoted === "strict";

Expand All @@ -196,10 +225,11 @@ function mangle_properties(ast, options) {
if (
node instanceof AST_ClassPrivateProperty
|| node instanceof AST_PrivateMethod
|| node instanceof AST_PrivateGetter
|| node instanceof AST_PrivateSetter
|| node instanceof AST_DotHash
) {
private_properties.add(node.key.name);
} else if (node instanceof AST_DotHash) {
private_properties.add(node.property);
// handled by mangle_private_properties
} else if (node instanceof AST_ObjectKeyVal) {
if (typeof node.key == "string" &&
(!keep_quoted_strict || !node.quote)) {
Expand Down Expand Up @@ -240,10 +270,11 @@ function mangle_properties(ast, options) {
if (
node instanceof AST_ClassPrivateProperty
|| node instanceof AST_PrivateMethod
|| node instanceof AST_PrivateGetter
|| node instanceof AST_PrivateSetter
|| node instanceof AST_DotHash
) {
node.key.name = mangle_private(node.key.name);
} else if (node instanceof AST_DotHash) {
node.property = mangle_private(node.property);
// handled by mangle_private_properties
} else if (node instanceof AST_ObjectKeyVal) {
if (typeof node.key == "string" &&
(!keep_quoted_strict || !node.quote)) {
Expand Down Expand Up @@ -324,16 +355,6 @@ function mangle_properties(ast, options) {
return mangled;
}

function mangle_private(name) {
let mangled = private_cache.get(name);
if (!mangled) {
mangled = nth_identifier.get(++cprivate);
private_cache.set(name, mangled);
}

return mangled;
}

function mangleStrings(node) {
return node.transform(new TreeTransformer(function(node) {
if (node instanceof AST_Sequence) {
Expand All @@ -353,4 +374,5 @@ function mangle_properties(ast, options) {
export {
reserve_quoted_keys,
mangle_properties,
mangle_private_properties,
};
2 changes: 2 additions & 0 deletions test/compress.js
View file
Expand Up @@ -14,6 +14,7 @@ import { Compressor } from "../lib/compress/index.js";
import {
reserve_quoted_keys,
mangle_properties,
mangle_private_properties,
} from "../lib/propmangle.js";
import { base54 } from "../lib/scope.js";
import { string_template, defaults } from "../lib/utils/index.js";
Expand Down Expand Up @@ -227,6 +228,7 @@ async function run_compress_tests() {
}
})(test.mangle.cache);
output.mangle_names(test.mangle);
mangle_private_properties(output, test.mangle);
if (test.mangle.properties) {
output = mangle_properties(output, test.mangle.properties);
}
Expand Down
10 changes: 7 additions & 3 deletions test/compress/class-properties.js
View file
Expand Up @@ -364,8 +364,10 @@ private_properties_can_be_mangled: {
#bbbbbb() {
return "SS"
}
get #cccccc() {}
set #dddddd(v) {}
log() {
console.log(this.aaaaaa + this.#aaaaaa + this.#bbbbbb())
console.log(this.aaaaaa + this.#aaaaaa + this.#bbbbbb() + this.#cccccc + this.#dddddd)
}
}

Expand All @@ -375,11 +377,13 @@ private_properties_can_be_mangled: {
class X {
t = "P"
#a = "A"
#b() {
#s() {
return "SS"
}
get #c() {}
set #t(a) {}
log() {
console.log(this.t + this.#a + this.#b())
console.log(this.t + this.#a + this.#s() + this.#c + this.#t)
}
}

Expand Down
30 changes: 26 additions & 4 deletions test/mocha/sourcemaps.js
View file
Expand Up @@ -137,23 +137,45 @@ describe("sourcemaps", function() {
it("Should grab names from methods and properties correctly", async () => {
const code = `class Foo {
property = 6
#private = 4
method () {}
404() {}
"quoted method name" () {}
get getter(){}
set setter(){}
#private = 4
#private_method() {}
get #private_getter() {}
set #private_setter() {}
test() {
this.property;
this.method;
this[404];
this["quoted method name"];
this.getter;
this.setter;
this.#private;
this.#private_method;
this.#private_getter;
this.#private_setter;
}
}`;
const result = await minify(code, {
sourceMap: {asObject: true},
sourceMap: { asObject: true },
mangle: { properties: true },
});
assert.deepStrictEqual(result.map.names, [
"Foo",
"property",
"private",
"method",
"getter",
"setter"
"setter",
"private",
"private_method",
"private_getter",
"private_setter",
"test",
"this",
]);
});

Expand Down

0 comments on commit e6312f4

Please sign in to comment.