implement mangle.properties.domprops (#5686)

- support destructuring syntax
- fix corner case with comma operator

README.md

@@ -891,12 +891,15 @@ UglifyJS.minify(code, { mangle: { toplevel: true } }).code;
 
 ### Mangle properties options
 
-- `builtins` (default: `false`) — Use `true` to allow the mangling of builtin
+- `builtins` (default: `false`) — Use `true` to allow the mangling of built-in
-  DOM properties. Not recommended to override this setting.
+  properties of JavaScript API. Not recommended to override this setting.
 
 - `debug` (default: `false`) — Mangle names with the original name still present.
   Pass an empty string `""` to enable, or a non-empty string to set the debug suffix.
 
+- `domprops` (default: `false`) — Use `true` to allow the mangling of properties
+  commonly found in Document Object Model. Not recommended to override this setting.
+
 - `keep_fargs` (default: `false`) — Use `true` to prevent mangling of function
   arguments.
 

bin/uglifyjs

@@ -238,17 +238,6 @@ if (specified["beautify"] && specified["output-opts"]) fatal("--beautify cannot
 [ "compress", "mangle" ].forEach(function(name) {
     if (!(name in options)) options[name] = false;
 });
-if (options.mangle && options.mangle.properties) {
-    if (options.mangle.properties.domprops) {
-        delete options.mangle.properties.domprops;
-    } else {
-        if (typeof options.mangle.properties != "object") options.mangle.properties = {};
-        if (!Array.isArray(options.mangle.properties.reserved)) options.mangle.properties.reserved = [];
-        require("../tools/domprops").forEach(function(name) {
-            UglifyJS.push_uniq(options.mangle.properties.reserved, name);
-        });
-    }
-}
 if (/^ast|spidermonkey$/.test(output)) {
     if (typeof options.output != "object") options.output = {};
     options.output.ast = true;

lib/propmangle.js

@@ -124,7 +124,9 @@ function get_builtins() {
 
 function reserve_quoted_keys(ast, reserved) {
     ast.walk(new TreeWalker(function(node) {
-        if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) {
+        if (node instanceof AST_ClassProperty
+            || node instanceof AST_DestructuredKeyVal
+            || node instanceof AST_ObjectProperty) {
             if (node.key instanceof AST_Node) {
                 addStrings(node.key, add);
             } else if (node.start && node.start.quote) {
@@ -158,12 +160,16 @@ function mangle_properties(ast, options) {
         builtins: false,
         cache: null,
         debug: false,
+        domprops: false,
         keep_quoted: false,
         regex: null,
         reserved: null,
     }, true);
 
     var reserved = options.builtins ? new Dictionary() : get_builtins();
+    if (!options.domprops && typeof domprops !== "undefined") domprops.forEach(function(name) {
+        reserved.set(name, true);
+    });
     if (Array.isArray(options.reserved)) options.reserved.forEach(function(name) {
         reserved.set(name, true);
     });
@@ -210,7 +216,9 @@ function mangle_properties(ast, options) {
                 addStrings(node.args[0], add);
                 break;
             }
-        } else if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) {
+        } else if (node instanceof AST_ClassProperty
+            || node instanceof AST_DestructuredKeyVal
+            || node instanceof AST_ObjectProperty) {
             if (node.key instanceof AST_Node) {
                 addStrings(node.key, add);
             } else {
@@ -244,7 +252,9 @@ function mangle_properties(ast, options) {
                 mangleStrings(node.args[0]);
                 break;
             }
-        } else if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) {
+        } else if (node instanceof AST_ClassProperty
+            || node instanceof AST_DestructuredKeyVal
+            || node instanceof AST_ObjectProperty) {
             if (node.key instanceof AST_Node) {
                 mangleStrings(node.key);
             } else {
@@ -307,7 +317,7 @@ function mangle_properties(ast, options) {
 
     function mangleStrings(node) {
         if (node instanceof AST_Sequence) {
-            mangleStrings(node.expressions.tail_node());
+            mangleStrings(node.tail_node());
         } else if (node instanceof AST_String) {
             node.value = mangle(node.value);
         } else if (node instanceof AST_Conditional) {

test/compress.js

@@ -271,7 +271,9 @@ function test_case(test) {
         expect = test.expect_exact;
     }
     var input = to_toplevel(test.input, test.mangle, test.expression);
-    var input_code = make_code(input, {}, test.expression);
+    var input_code = make_code(input, {
+        keep_quoted_props: true,
+    }, test.expression);
     var input_formatted = make_code(test.input, {
         annotations: true,
         beautify: true,

test/compress/classes.js

@@ -2164,6 +2164,7 @@ issue_4829_2: {
 mangle_properties: {
     mangle = {
         properties: {
+            domprops: true,
             keep_quoted: true,
         },
     }

test/compress/destructured.js

@@ -1737,6 +1737,23 @@ singleton_side_effects: {
     node_version: ">=6"
 }
 
+mangle_properties: {
+    mangle = {
+        properties: {
+            domprops: true,
+        },
+    }
+    input: {
+        function f({ p: a }) {
+            return a;
+        }
+        console.log(f({ p: "PASS" }));
+    }
+    expect_exact: 'function f({n}){return n}console.log(f({n:"PASS"}));'
+    expect_stdout: "PASS"
+    node_version: ">=6"
+}
+
 issue_4280: {
     options = {
         evaluate: true,

test/compress/issue-1321.js

@@ -1,6 +1,7 @@
 issue_1321_no_debug: {
     mangle = {
         properties: {
+            domprops: true,
             keep_quoted: true,
         },
     }
@@ -23,6 +24,7 @@ issue_1321_debug: {
     mangle = {
         properties: {
             debug: "",
+            domprops: true,
             keep_quoted: true,
         },
     }
@@ -44,6 +46,7 @@ issue_1321_debug: {
 issue_1321_with_quoted: {
     mangle = {
         properties: {
+            domprops: true,
             keep_quoted: false,
         },
     }

test/compress/issue-1770.js

@@ -65,7 +65,9 @@ mangle_props: {
 
 numeric_literal: {
     mangle = {
-        properties: true,
+        properties: {
+            domprops: true,
+        },
     }
     beautify = {
         beautify: true,
@@ -125,6 +127,7 @@ identifier: {
     mangle = {
         properties: {
             builtins: true,
+            domprops: true,
         },
     }
     input: {

test/compress/issue-747.js

@@ -1,6 +1,7 @@
 dont_reuse_prop: {
     mangle = {
         properties: {
+            domprops: true,
             regex: /asd/,
         },
     }
@@ -29,6 +30,7 @@ dont_reuse_prop: {
 unmangleable_props_should_always_be_reserved: {
     mangle = {
         properties: {
+            domprops: true,
             regex: /asd/,
         },
     }

test/compress/objects.js

@@ -173,7 +173,9 @@ numeric_literal: {
         side_effects: true,
     }
     mangle = {
-        properties: true,
+        properties: {
+            domprops: true,
+        },
     }
     beautify = {
         beautify: true,

test/compress/properties.js

@@ -133,6 +133,7 @@ evaluate_string_length: {
 mangle_properties_1: {
     mangle = {
         properties: {
+            domprops: true,
             keep_quoted: false,
         },
     }
@@ -155,9 +156,10 @@ mangle_properties_1: {
 mangle_properties_2: {
     mangle = {
         properties: {
+            domprops: true,
             reserved: [
                 "value",
-            ]
+            ],
         },
     }
     input: {
@@ -199,6 +201,24 @@ mangle_properties_2: {
     ]
 }
 
+mangle_properties_3: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        console.log({
+            [(console, "foo")]: "PASS",
+        }.foo);
+    }
+    expect: {
+        console.log({
+            [(console, "o")]: "PASS",
+        }.o);
+    }
+    expect_stdout: "PASS"
+    node_version: ">=4"
+}
+
 mangle_unquoted_properties: {
     options = {
         evaluate: true,
@@ -207,6 +227,7 @@ mangle_unquoted_properties: {
     mangle = {
         properties: {
             builtins: true,
+            domprops: true,
             keep_quoted: true,
         },
     }
@@ -308,6 +329,7 @@ mangle_debug_suffix_keep_quoted: {
         properties: {
             builtins: true,
             debug: "XYZ",
+            domprops: true,
             keep_quoted: true,
             reserved: [],
         },

test/mocha/let.js

@@ -42,7 +42,9 @@ describe("let", function() {
             compress: false,
             ie: true,
             mangle: {
-                properties: true,
+                properties: {
+                    domprops: true,
+                },
             },
         });
         if (result.error) throw result.error;

test/mocha/minify.js

@@ -110,10 +110,12 @@ describe("minify", function() {
             var result = UglifyJS.minify(code, {
                 compress: false,
                 mangle: {
-                    properties: true,
+                    properties: {
-                    toplevel: true
+                        domprops: true,
+                    },
+                    toplevel: true,
                 },
-                nameCache: cache
+                nameCache: cache,
             });
             if (result.error) throw result.error;
             original += code;
@@ -188,21 +190,19 @@ describe("minify", function() {
         it("Shouldn't mangle quoted properties", function() {
             var js = 'a["foo"] = "bar"; a.color = "red"; x = {"bar": 10};';
             var result = UglifyJS.minify(js, {
-                compress: {
+                compress: true,
-                    properties: false
-                },
                 mangle: {
                     properties: {
-                        keep_quoted: true
+                        domprops: true,
-                    }
+                        keep_quoted: true,
+                    },
                 },
                 output: {
                     keep_quoted_props: true,
-                    quote_style: 3
+                    quote_style: 3,
-                }
+                },
             });
-            assert.strictEqual(result.code,
+            assert.strictEqual(result.code, 'a["foo"]="bar",a.a="red",x={"bar":10};');
-                    'a["foo"]="bar",a.a="red",x={"bar":10};');
         });
         it("Should not mangle quoted property within dead code", function() {
             var result = UglifyJS.minify('({ "keep": 1 }); g.keep = g.change = 42;', {

test/node.js

@@ -1,6 +1,6 @@
 var fs = require("fs");
 
-new Function("exports", require("../tools/node").FILES.map(function(file) {
+new Function("domprops", "exports", require("../tools/node").FILES.map(function(file) {
     if (/exports\.js$/.test(file)) file = require.resolve("./exports");
     return fs.readFileSync(file, "utf8");
-}).join("\n\n"))(exports);
+}).join("\n\n"))(require("../tools/domprops.json"), exports);

tools/node.js

@@ -15,13 +15,13 @@ exports.FILES = [
     require.resolve("./exports.js"),
 ];
 
-new Function("exports", function() {
+new Function("domprops", "exports", function() {
     var code = exports.FILES.map(function(file) {
         return fs.readFileSync(file, "utf8");
     });
     code.push("exports.describe_ast = " + describe_ast.toString());
     return code.join("\n\n");
-}())(exports);
+}())(require("./domprops.json"), exports);
 
 function to_comment(value) {
     if (typeof value != "string") value = JSON.stringify(value, function(key, value) {