From bb4221c7e7376923782dcbaae618cc18f4b010b4 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Wed, 25 Jan 2017 23:50:19 +0100 Subject: [PATCH] Mark renamed paths in mangler The issue was renaming an already renamed path. Maybe something quircky about bindings of destructuring assingments in babel. + (Fix #326) + (Fix #369) --- .../__tests__/mangle-names-test.js | 38 +++++++++++++++++++ .../src/index.js | 24 ++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/packages/babel-plugin-minify-mangle-names/__tests__/mangle-names-test.js b/packages/babel-plugin-minify-mangle-names/__tests__/mangle-names-test.js index 21e6888fd..1d7446305 100644 --- a/packages/babel-plugin-minify-mangle-names/__tests__/mangle-names-test.js +++ b/packages/babel-plugin-minify-mangle-names/__tests__/mangle-names-test.js @@ -1065,4 +1065,42 @@ describe("mangle-names", () => { expect(transform(source, { topLevel: true })).toBe(expected); }); + + it("should fix #326, #369 - destructuring", () => { + const source = unpad(` + // issue#326 + function a() { + let foo, bar, baz; + ({foo, bar, baz} = {}); + return {foo, bar, baz}; + } + // issue#369 + function decodeMessage(message){ + let namespace; + let name; + let value = null; + + [, namespace, name, value] = message.split(',') || []; + console.log(name); + } + `); + const expected = unpad(` + // issue#326 + function a() { + let b, c, d; + ({ foo: b, bar: c, baz: d } = {}); + return { foo: b, bar: c, baz: d }; + } + // issue#369 + function decodeMessage(b) { + let c; + let d; + let e = null; + + [, c, d, e] = b.split(\',\') || []; + console.log(d); + } + `); + expect(transform(source)).toBe(expected); + }); }); diff --git a/packages/babel-plugin-minify-mangle-names/src/index.js b/packages/babel-plugin-minify-mangle-names/src/index.js index e55bcd474..bb9c435a7 100644 --- a/packages/babel-plugin-minify-mangle-names/src/index.js +++ b/packages/babel-plugin-minify-mangle-names/src/index.js @@ -6,6 +6,8 @@ const { hasEval, } = require("babel-helper-mark-eval-scopes"); +const PATH_RENAME_MARKER = Symbol("PATH_RENAME_MARKER"); + module.exports = ({ types: t, traverse }) => { const hop = Object.prototype.hasOwnProperty; @@ -181,7 +183,16 @@ module.exports = ({ types: t, traverse }) => { const binding = scope.getBinding(oldName); // rename at the declaration level - binding.identifier.name = newName; + const bindingPaths = binding.path.getBindingIdentifierPaths(); + + Object + .keys(bindingPaths) + .map((b) => { + if (b === oldName && !bindingPaths[b][PATH_RENAME_MARKER]) { + bindingPaths[b].replaceWith(t.identifier(newName)); + bindingPaths[b][PATH_RENAME_MARKER] = true; + } + }); const {bindings} = scope; bindings[newName] = binding; @@ -192,11 +203,14 @@ module.exports = ({ types: t, traverse }) => { for (let i = 0; i < violations.length; i++) { if (violations[i].isLabeledStatement()) continue; - const bindings = violations[i].getBindingIdentifiers(); + const bindings = violations[i].getBindingIdentifierPaths(); Object .keys(bindings) .map((b) => { - bindings[b].name = newName; + if (b === oldName && !bindings[b][PATH_RENAME_MARKER]) { + bindings[b].replaceWith(t.identifier(newName)); + bindings[b][PATH_RENAME_MARKER] = true; + } }); } @@ -204,6 +218,8 @@ module.exports = ({ types: t, traverse }) => { const refs = binding.referencePaths; for (let i = 0; i < refs.length; i++) { const path = refs[i]; + if (path[PATH_RENAME_MARKER]) continue; + const {node} = path; if (!path.isIdentifier()) { // Ideally, this should not happen @@ -216,7 +232,7 @@ module.exports = ({ types: t, traverse }) => { // replacement in dce from `x` to `!x` gives referencePath as `!x` path.traverse({ ReferencedIdentifier(refPath) { - if (refPath.node.name === oldName && refPath.scope === scope) { + if (refPath.node.name === oldName && refPath.scope === scope && !refPath[PATH_RENAME_MARKER]) { refPath.node.name = newName; } }