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 0eab4a8cc..57ed4add9 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 @@ -1129,4 +1129,24 @@ describe("mangle-names", () => { `); expect(transform(source)).toBe(expected); }); + + it("should fix issue#365 - classDeclaration with unsafe parent scope", () => { + const source = unpad(` + function foo() { + eval(""); + class A {} + class B {} + } + `); + expect(transform(source)).toBe(source); + }); + + it("should fix classDeclaration with unsafe program scope", () => { + const source = unpad(` + class A {} + class B {} + eval(""); + `); + expect(transform(source, { topLevel: true })).toBe(source); + }); }); diff --git a/packages/babel-plugin-minify-mangle-names/src/index.js b/packages/babel-plugin-minify-mangle-names/src/index.js index 26e650bf6..d6b8a520d 100644 --- a/packages/babel-plugin-minify-mangle-names/src/index.js +++ b/packages/babel-plugin-minify-mangle-names/src/index.js @@ -86,100 +86,91 @@ module.exports = ({ types: t, traverse }) => { } } - mangle() { + mangleScope(scope) { const mangler = this; - this.program.traverse({ - Scopable(path) { - const {scope} = path; + if (!mangler.eval && hasEval(scope)) return; - if (!mangler.eval && hasEval(scope)) return; + if (mangler.visitedScopes.has(scope)) return; + mangler.visitedScopes.add(scope); - if (mangler.visitedScopes.has(scope)) return; - mangler.visitedScopes.add(scope); + let i = 0; + function getNext() { + return mangler.charset.getIdentifier(i++); + } - function hasOwnBinding(name) { - if (scope.parent !== mangler.program.scope) { - return scope.hasOwnBinding(name); - } - return mangler.program.scope.hasOwnBinding(name) || scope.hasOwnBinding(name); - } + // This is useful when we have vars of single character + // => var a, ...z, A, ...Z, $, _; + // to + // => var aa, a, b ,c; + // instead of + // => var aa, ab, ...; + // TODO: + // Re-enable after enabling this feature + // This doesn't work right now as we are concentrating + // on performance improvements + // function resetNext() { + // i = 0; + // } + + const bindings = scope.getAllBindings(); + const names = Object.keys(bindings); + + for (let i = 0; i < names.length; i++) { + const oldName = names[i]; + const binding = bindings[oldName]; + + if ( + // arguments + oldName === "arguments" + // other scope bindings + || !scope.hasOwnBinding(oldName) + // labels + || binding.path.isLabeledStatement() + // ClassDeclaration has binding in two scopes + // 1. The scope in which it is declared + // 2. The class's own scope + // - https://github.com/babel/babel/issues/5156 + || (binding.path.isClassDeclaration() && binding.path === scope.path) + // blacklisted + || mangler.isBlacklist(oldName) + // function names + || (mangler.keepFnName ? isFunction(binding.path) : false) + // class names + || (mangler.keepClassName ? isClass(binding.path) : false) + ) { + continue; + } - let i = 0; - function getNext() { - return mangler.charset.getIdentifier(i++); - } + let next; + do { + next = getNext(); + } while ( + !t.isValidIdentifier(next) + || hop.call(bindings, next) + || scope.hasGlobal(next) + || scope.hasReference(next) + ); + + // TODO: + // re-enable this - check above + // resetNext(); + mangler.rename(scope, oldName, next); + } + } - // This is useful when we have vars of single character - // => var a, ...z, A, ...Z, $, _; - // to - // => var aa, a, b ,c; - // instead of - // => var aa, ab, ...; - // TODO: - // Re-enable after enabling this feature - // This doesn't work right now as we are concentrating - // on performance improvements - // function resetNext() { - // i = 0; - // } - - const bindings = scope.getAllBindings(); - const names = Object.keys(bindings); - - for (let i = 0; i < names.length; i++) { - const oldName = names[i]; - const binding = bindings[oldName]; - const isTopLevel = mangler.program.scope.bindings[oldName] === binding; - - if ( - // already renamed bindings - binding.renamed - // arguments - || oldName === "arguments" - // globals - || (mangler.topLevel ? false : isTopLevel) - // other scope bindings - || !hasOwnBinding(oldName) - // labels - || binding.path.isLabeledStatement() - // blacklisted - || mangler.isBlacklist(oldName) - // function names - || (mangler.keepFnName ? isFunction(binding.path) : false) - // class names - || (mangler.keepClassName ? isClass(binding.path) : false) - ) { - continue; - } + mangle() { + const mangler = this; - let next; - do { - next = getNext(); - } while ( - !t.isValidIdentifier(next) - || hop.call(bindings, next) - || scope.hasGlobal(next) - || scope.hasReference(next) - ); - - // TODO: - // re-enable this - check above - // resetNext(); - mangler.rename(scope, oldName, next); - if (isTopLevel) { - mangler.rename(mangler.program.scope, oldName, next); - } - // mark the binding as renamed - binding.renamed = true; - } + if (mangler.topLevel) { + mangler.mangleScope(mangler.program.scope); + } + + this.program.traverse({ + Scopable(path) { + mangler.mangleScope(path.scope); } }); - - // TODO: - // re-enable - // check above - // this.updateReferences(); } rename(scope, oldName, newName) {