Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix private getter/setter mangling #1060

Merged
merged 7 commits into from Sep 5, 2021
Merged

Fix private getter/setter mangling #1060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6c0055b
Mangle private properties by default when manglin
jridgewell Aug 31, 2021
d9027a6
Fix sourcemap test for private fields
jridgewell Aug 31, 2021
bcc63c5
Fix property mangling private getter/setter
jridgewell Aug 31, 2021
dbbf749
tmp
jridgewell Aug 31, 2021
cc28a6e
Improve sourcemap output for private properties
jridgewell Aug 31, 2021
686334b
remove test grep
jridgewell Aug 31, 2021
71d92b8
Don't print regular string literals
jridgewell Sep 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/ast.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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