From 6c0055b61233345e93b6a53c603c771ca6a73024 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Mon, 30 Aug 2021 21:57:47 -0400 Subject: [PATCH 1/7] Mangle private properties by default when manglin --- lib/minify.js | 2 ++ lib/propmangle.js | 51 +++++++++++++++++++++++-------------- terser-functional-tests | 1 + test/compress.js | 3 +++ test/compress/issue_1014.js | 6 ++--- 5 files changed, 41 insertions(+), 22 deletions(-) create mode 160000 terser-functional-tests diff --git a/lib/minify.js b/lib/minify.js index 8ba1848a6..d3c501fee 100644 --- a/lib/minify.js +++ b/lib/minify.js @@ -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"; @@ -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) { diff --git a/lib/propmangle.js b/lib/propmangle.js index 374c045ab..904db4bcd 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -140,6 +140,33 @@ 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; + + return ast.transform(new TreeTransformer(function(node) { + 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); + } + })); + + 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, @@ -161,10 +188,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) { @@ -187,7 +212,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"; @@ -196,10 +220,9 @@ function mangle_properties(ast, options) { if ( node instanceof AST_ClassPrivateProperty || node instanceof AST_PrivateMethod + || 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)) { @@ -240,10 +263,9 @@ function mangle_properties(ast, options) { if ( node instanceof AST_ClassPrivateProperty || node instanceof AST_PrivateMethod + || 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)) { @@ -324,16 +346,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) { @@ -353,4 +365,5 @@ function mangle_properties(ast, options) { export { reserve_quoted_keys, mangle_properties, + mangle_private_properties, }; diff --git a/terser-functional-tests b/terser-functional-tests new file mode 160000 index 000000000..7e4474588 --- /dev/null +++ b/terser-functional-tests @@ -0,0 +1 @@ +Subproject commit 7e44745883a4c4f6f739aa7fc414468b6f3898a8 diff --git a/test/compress.js b/test/compress.js index dd18e06db..12df0b429 100644 --- a/test/compress.js +++ b/test/compress.js @@ -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"; @@ -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); } @@ -291,6 +293,7 @@ async function run_compress_tests() { } var tests = parse_test(path.resolve(dir, file)); for (var i in tests) if (tests.hasOwnProperty(i)) { + if (process.env.GREP && !i.includes(process.env.GREP)) continue; if (!await test_case(tests[i])) { failures++; failed_files[file] = 1; diff --git a/test/compress/issue_1014.js b/test/compress/issue_1014.js index 5c0175b05..bae45e349 100644 --- a/test/compress/issue_1014.js +++ b/test/compress/issue_1014.js @@ -74,10 +74,10 @@ ternary_and_private_methods: { } expect: { class A { - #fail() { return false; } - get #pass() { return "PASS"; } + #s() { return false; } + get #i() { return "PASS"; } print() { - console.log(this.#fail() ? this.#fail() : this.#pass); + console.log(this.#s() ? this.#s() : this.#i); } } new A().print(); From d9027a66f0ac53db27bdb603848dcac2c1da2c60 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Mon, 30 Aug 2021 22:49:04 -0400 Subject: [PATCH 2/7] Fix sourcemap test for private fields --- lib/ast.js | 2 +- test/mocha/sourcemaps.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ast.js b/lib/ast.js index 3aca3dd77..c0a50d437 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -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); diff --git a/test/mocha/sourcemaps.js b/test/mocha/sourcemaps.js index c1035c1d7..8aae876ad 100644 --- a/test/mocha/sourcemaps.js +++ b/test/mocha/sourcemaps.js @@ -145,6 +145,7 @@ describe("sourcemaps", function() { set setter(){} }`; const result = await minify(code, { + mangle: false, sourceMap: {asObject: true}, }); assert.deepStrictEqual(result.map.names, [ From bcc63c5f22509d01526155aa859aa470ce601daf Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Mon, 30 Aug 2021 23:30:40 -0400 Subject: [PATCH 3/7] Fix property mangling private getter/setter --- lib/propmangle.js | 11 ++++++++++- test/compress/class-properties.js | 10 +++++++--- test/compress/issue_1014.js | 6 +++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/propmangle.js b/lib/propmangle.js index 904db4bcd..6f1a55a47 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -59,6 +59,8 @@ import { AST_ObjectKeyVal, AST_ObjectProperty, AST_PrivateMethod, + AST_PrivateGetter, + AST_PrivateSetter, AST_Sequence, AST_String, AST_Sub, @@ -145,16 +147,19 @@ function mangle_private_properties(ast, options) { var private_cache = new Map(); var nth_identifier = options.nth_identifier || base54; - return ast.transform(new TreeTransformer(function(node) { + 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); @@ -220,6 +225,8 @@ 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 ) { // handled by mangle_private_properties @@ -263,6 +270,8 @@ 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 ) { // handled by mangle_private_properties diff --git a/test/compress/class-properties.js b/test/compress/class-properties.js index 5393690d3..290aef881 100644 --- a/test/compress/class-properties.js +++ b/test/compress/class-properties.js @@ -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) } } @@ -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) } } diff --git a/test/compress/issue_1014.js b/test/compress/issue_1014.js index bae45e349..5c0175b05 100644 --- a/test/compress/issue_1014.js +++ b/test/compress/issue_1014.js @@ -74,10 +74,10 @@ ternary_and_private_methods: { } expect: { class A { - #s() { return false; } - get #i() { return "PASS"; } + #fail() { return false; } + get #pass() { return "PASS"; } print() { - console.log(this.#s() ? this.#s() : this.#i); + console.log(this.#fail() ? this.#fail() : this.#pass); } } new A().print(); From dbbf749c7a4e60abdb376fc24b759b9350c67b27 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 31 Aug 2021 02:07:20 -0400 Subject: [PATCH 4/7] tmp --- terser-functional-tests | 1 - 1 file changed, 1 deletion(-) delete mode 160000 terser-functional-tests diff --git a/terser-functional-tests b/terser-functional-tests deleted file mode 160000 index 7e4474588..000000000 --- a/terser-functional-tests +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 7e44745883a4c4f6f739aa7fc414468b6f3898a8 From cc28a6e5235d6f5bfbbf48a4c7e4cbe7cad3c092 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 31 Aug 2021 02:06:11 -0400 Subject: [PATCH 5/7] Improve sourcemap output for private properties --- lib/output.js | 11 ++++++++--- test/mocha/sourcemaps.js | 31 ++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/lib/output.js b/lib/output.js index 382810b77..d8370fb32 100644 --- a/lib/output.js +++ b/lib/output.js @@ -349,8 +349,10 @@ 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) { + let { name, token } = mapping; + if (token.type == "name" || token.type === "privatename" || token.type === "string") { + name = token.value; + } else if (name instanceof AST_Symbol) { name = name.name; } options.source_map.add( @@ -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) { @@ -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) { diff --git a/test/mocha/sourcemaps.js b/test/mocha/sourcemaps.js index 8aae876ad..cb8f31215 100644 --- a/test/mocha/sourcemaps.js +++ b/test/mocha/sourcemaps.js @@ -137,24 +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, { - mangle: false, - 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", ]); }); From 686334bd825621eefdc29d4c1697a0e44dfe6786 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 31 Aug 2021 02:13:14 -0400 Subject: [PATCH 6/7] remove test grep --- test/compress.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/compress.js b/test/compress.js index 12df0b429..4c33f90d8 100644 --- a/test/compress.js +++ b/test/compress.js @@ -293,7 +293,6 @@ async function run_compress_tests() { } var tests = parse_test(path.resolve(dir, file)); for (var i in tests) if (tests.hasOwnProperty(i)) { - if (process.env.GREP && !i.includes(process.env.GREP)) continue; if (!await test_case(tests[i])) { failures++; failed_files[file] = 1; From 71d92b8b75cf1e83af7cd5133e3d34f2ba276832 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Thu, 2 Sep 2021 04:04:21 -0400 Subject: [PATCH 7/7] Don't print regular string literals --- lib/output.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/output.js b/lib/output.js index d8370fb32..c85b61b46 100644 --- a/lib/output.js +++ b/lib/output.js @@ -350,10 +350,10 @@ function OutputStream(options) { mappings.forEach(function(mapping) { try { let { name, token } = mapping; - if (token.type == "name" || token.type === "privatename" || token.type === "string") { + if (token.type == "name" || token.type === "privatename") { name = token.value; } else if (name instanceof AST_Symbol) { - name = name.name; + name = token.type === "string" ? token.value : name.name; } options.source_map.add( mapping.token.file,