Skip to content

Commit

Permalink
Mark renamed paths in mangler (#390)
Browse files Browse the repository at this point in the history
The issue was renaming an already renamed path. Maybe something quircky about bindings of destructuring assingments in babel.

+ (Fix #326)
+ (Fix #369)
  • Loading branch information
boopathi authored and kangax committed Jan 30, 2017
1 parent 9c016a3 commit 39b74de
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
Expand Up @@ -1067,4 +1067,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);
});
});
24 changes: 20 additions & 4 deletions packages/babel-plugin-minify-mangle-names/src/index.js
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -192,18 +203,23 @@ 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;
}
});
}

// update all referenced places
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
Expand All @@ -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;
}
}
Expand Down

0 comments on commit 39b74de

Please sign in to comment.