Skip to content

Commit

Permalink
Don't look up variables past hoisting scope
Browse files Browse the repository at this point in the history 
When we are renaming variables - we don't need to check for the
conflicts past the closest scope that stops the hoisting. Additionally,
function expression's name should be put into function's scope, and not
outside of the function.

Fix: evanw#1147
  • Loading branch information
@indutny @indutny-signal
indutny authored and indutny-signal committed Feb 26, 2022
1 parent 4db57c1 commit 2ce7687
Show file tree
Hide file tree
Showing 10 changed files with 408 additions and 298 deletions.
63 changes: 63 additions & 0 deletions internal/bundler/bundler_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,69 @@ func TestNestedScopeBug(t *testing.T) {
})
}

// This test checks that function expression's name is placed in correct scope
// and not changed by NumberRenamer, as well as function arguments not clashing
// with the top-level scope and not being renamed
func TestRenamerFnExprName(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
export const a = function a(b) {
return b;
}
export const b = function b(a) {
return a;
}
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

// Here we verify that conflicting exports are properly renamed when bundled.
func TestRenamerBundleConflicts(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
export { Foo, makeFoo } from './a';
export { Foo as Bar, makeFoo as makeBar } from './b';
`,
"/a.js": `
export class Foo {
constructor() {
this.name = 'a';
}
}
export function makeFoo() {
return new Foo('a');
}
`,
"/b.js": `
export class Foo {
constructor() {
this.name = 'b';
}
}
export function makeFoo() {
return new Foo('b');
}
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestHashbangBundle(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion internal/bundler/snapshots/snapshots_dce.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ function nested() {

---------- /out/namespace-export.js ----------
var ns;
((ns2) => (ns2.y_keep = 2, console.log(1, 1, 2, 2)))(ns ||= {});
((ns) => (ns.y_keep = 2, console.log(1, 1, 2, 2)))(ns ||= {});

---------- /out/comment-before.js ----------
{
Expand Down
90 changes: 67 additions & 23 deletions internal/bundler/snapshots/snapshots_default.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ console.log(foo, out, sha256, config);
TestFalseRequire
---------- /out.js ----------
// entry.js
((require2) => require2("/test.txt"))();
((require) => require("/test.txt"))();

================================================================================
TestHashbangBundle
Expand Down Expand Up @@ -2580,8 +2580,8 @@ TestRequireMainCacheCommonJS
---------- /out.js ----------
// is-main.js
var require_is_main = __commonJS({
"is-main.js"(exports2, module2) {
module2.exports = require.main === module2;
"is-main.js"(exports, module) {
module.exports = require.main === module;
}
});

Expand Down Expand Up @@ -3268,7 +3268,7 @@ var require_es6_import_stmt = __commonJS({
// es6-import-assign.ts
var require_es6_import_assign = __commonJS({
"es6-import-assign.ts"(exports) {
var x2 = (init_dummy(), __toCommonJS(dummy_exports));
var x = (init_dummy(), __toCommonJS(dummy_exports));
console.log(exports);
}
});
Expand Down Expand Up @@ -3301,8 +3301,8 @@ var require_es6_export_assign = __commonJS({
var require_es6_ns_export_variable = __commonJS({
"es6-ns-export-variable.ts"(exports) {
var ns;
((ns2) => {
ns2.foo = 123;
((ns) => {
ns.foo = 123;
})(ns || (ns = {}));
console.log(exports);
}
Expand All @@ -3312,10 +3312,10 @@ var require_es6_ns_export_variable = __commonJS({
var require_es6_ns_export_function = __commonJS({
"es6-ns-export-function.ts"(exports) {
var ns;
((ns2) => {
((ns) => {
function foo() {
}
ns2.foo = foo;
ns.foo = foo;
})(ns || (ns = {}));
console.log(exports);
}
Expand All @@ -3325,10 +3325,10 @@ var require_es6_ns_export_function = __commonJS({
var require_es6_ns_export_async_function = __commonJS({
"es6-ns-export-async-function.ts"(exports) {
var ns;
((ns2) => {
((ns) => {
async function foo() {
}
ns2.foo = foo;
ns.foo = foo;
})(ns || (ns = {}));
console.log(exports);
}
Expand All @@ -3338,10 +3338,10 @@ var require_es6_ns_export_async_function = __commonJS({
var require_es6_ns_export_enum = __commonJS({
"es6-ns-export-enum.ts"(exports) {
var ns;
((ns2) => {
((ns) => {
let Foo;
((Foo2) => {
})(Foo = ns2.Foo || (ns2.Foo = {}));
((Foo) => {
})(Foo = ns.Foo || (ns.Foo = {}));
})(ns || (ns = {}));
console.log(exports);
}
Expand All @@ -3351,10 +3351,10 @@ var require_es6_ns_export_enum = __commonJS({
var require_es6_ns_export_const_enum = __commonJS({
"es6-ns-export-const-enum.ts"(exports) {
var ns;
((ns2) => {
((ns) => {
let Foo;
((Foo2) => {
})(Foo = ns2.Foo || (ns2.Foo = {}));
((Foo) => {
})(Foo = ns.Foo || (ns.Foo = {}));
})(ns || (ns = {}));
console.log(exports);
}
Expand All @@ -3378,10 +3378,10 @@ var require_es6_ns_export_namespace = __commonJS({
var require_es6_ns_export_class = __commonJS({
"es6-ns-export-class.ts"(exports) {
var ns;
((ns2) => {
((ns) => {
class Foo {
}
ns2.Foo = Foo;
ns.Foo = Foo;
})(ns || (ns = {}));
console.log(exports);
}
Expand All @@ -3391,10 +3391,10 @@ var require_es6_ns_export_class = __commonJS({
var require_es6_ns_export_abstract_class = __commonJS({
"es6-ns-export-abstract-class.ts"(exports) {
var ns;
((ns2) => {
((ns) => {
class Foo {
}
ns2.Foo = Foo;
ns.Foo = Foo;
})(ns || (ns = {}));
console.log(exports);
}
Expand Down Expand Up @@ -3719,7 +3719,7 @@ var Foo3 = class {

// read-setter.js
var Foo4 = class {
set #foo(x2) {
set #foo(x) {
}
foo() {
return this.#foo;
Expand All @@ -3728,7 +3728,7 @@ var Foo4 = class {

// node_modules/read-setter.js
var Foo5 = class {
set #foo(x2) {
set #foo(x) {
}
foo() {
return this.#foo;
Expand All @@ -3737,7 +3737,7 @@ var Foo5 = class {

// plugin-dir/node_modules/read-setter.js
var Foo6 = class {
set #foo(x2) {
set #foo(x) {
}
foo() {
return this.#foo;
Expand Down Expand Up @@ -3789,3 +3789,47 @@ TestWithStatementTaintingNoBundle
outerDead++;
}
})();

================================================================================
TestRenamerFnExprName
---------- /out.js ----------
// entry.js
var a = function a(b) {
return b;
};
var b = function b(a) {
return a;
};
export {
a,
b
};

================================================================================
TestRenamerBundleConflicts
---------- /out.js ----------
// a.js
var Foo = class {
constructor() {
this.name = "a";
}
};
function makeFoo() {
return new Foo("a");
}

// b.js
var Foo2 = class {
constructor() {
this.name = "b";
}
};
function makeFoo2() {
return new Foo2("b");
}
export {
Foo2 as Bar,
Foo,
makeFoo2 as makeBar,
makeFoo
};
72 changes: 36 additions & 36 deletions internal/bundler/snapshots/snapshots_loader.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,10 +546,10 @@ var require_cjs = __commonJS({
// entry-cjs.js
var require_entry_cjs = __commonJS({
"entry-cjs.js"(exports) {
var { b: esm_foo_2 } = (init_esm(), __toCommonJS(esm_exports));
var { b: esm_foo_ } = (init_esm(), __toCommonJS(esm_exports));
var { a: cjs_foo_ } = require_cjs();
exports.c = [
esm_foo_2,
esm_foo_,
cjs_foo_
];
}
Expand Down Expand Up @@ -721,29 +721,29 @@ console.log(foo.KEEP_FIELD, foo.a);

---------- /out/namespace-exports.js ----------
var ns;
((ns2) => {
ns2.b = 1;
ns2.c = 2;
ns2.d = 3;
({ i: { a: ns2.a } } = 4);
((ns) => {
ns.b = 1;
ns.c = 2;
ns.d = 3;
({ i: { a: ns.a } } = 4);
function MANGLE_FUNCTION_() {
}
ns2.g = MANGLE_FUNCTION_;
ns.g = MANGLE_FUNCTION_;
class MANGLE_CLASS_ {
}
ns2.h = MANGLE_CLASS_;
ns.h = MANGLE_CLASS_;
let MANGLE_NAMESPACE_;
((MANGLE_NAMESPACE_2) => {
((MANGLE_NAMESPACE_) => {
;
})(MANGLE_NAMESPACE_ = ns2.e || (ns2.e = {}));
})(MANGLE_NAMESPACE_ = ns.e || (ns.e = {}));
let MANGLE_ENUM_;
((MANGLE_ENUM_2) => {
})(MANGLE_ENUM_ = ns2.f || (ns2.f = {}));
((MANGLE_ENUM_) => {
})(MANGLE_ENUM_ = ns.f || (ns.f = {}));
console.log({
VAR: ns2.b,
LET: ns2.c,
CONST: ns2.d,
DESTRUCTURING: ns2.a,
VAR: ns.b,
LET: ns.c,
CONST: ns.d,
DESTRUCTURING: ns.a,
FUNCTION: MANGLE_FUNCTION_,
CLASS: MANGLE_CLASS_,
NAMESPACE: MANGLE_NAMESPACE_,
Expand All @@ -760,40 +760,40 @@ console.log({
NAMESPACE: ns.e,
ENUM: ns.f
});
((ns2) => {
((ns) => {
console.log({
VAR: ns2.b,
LET: ns2.c,
CONST: ns2.d,
DESTRUCTURING: ns2.a,
FUNCTION: ns2.g,
CLASS: ns2.h,
NAMESPACE: ns2.e,
ENUM: ns2.f
VAR: ns.b,
LET: ns.c,
CONST: ns.d,
DESTRUCTURING: ns.a,
FUNCTION: ns.g,
CLASS: ns.h,
NAMESPACE: ns.e,
ENUM: ns.f
});
})(ns || (ns = {}));

---------- /out/enum-values.js ----------
var TopLevelNumber = /* @__PURE__ */ ((TopLevelNumber2) => {
TopLevelNumber2[TopLevelNumber2["foo_"] = 0] = "foo_";
return TopLevelNumber2;
var TopLevelNumber = /* @__PURE__ */ ((TopLevelNumber) => {
TopLevelNumber[TopLevelNumber["foo_"] = 0] = "foo_";
return TopLevelNumber;
})(TopLevelNumber || {});
var TopLevelString = /* @__PURE__ */ ((TopLevelString2) => {
TopLevelString2["bar_"] = "";
return TopLevelString2;
var TopLevelString = /* @__PURE__ */ ((TopLevelString) => {
TopLevelString["bar_"] = "";
return TopLevelString;
})(TopLevelString || {});
console.log({
foo: TopLevelNumber.a,
bar: TopLevelString.b
});
function fn() {
let NestedNumber;
((NestedNumber2) => {
NestedNumber2[NestedNumber2["foo_"] = 0] = "foo_";
((NestedNumber) => {
NestedNumber[NestedNumber["foo_"] = 0] = "foo_";
})(NestedNumber || (NestedNumber = {}));
let NestedString;
((NestedString2) => {
NestedString2["bar_"] = "";
((NestedString) => {
NestedString["bar_"] = "";
})(NestedString || (NestedString = {}));
console.log({
foo: TopLevelNumber.a,
Expand Down
Loading

0 comments on commit 2ce7687

Please sign in to comment.