Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for binding shadowing outer var with loop closure #15309

Merged
merged 2 commits into from Dec 23, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 14 additions & 4 deletions packages/babel-plugin-transform-block-scoping/src/index.ts
Expand Up @@ -71,22 +71,32 @@ export default declare((api, opts: Options) => {

if (headPath && isBlockScoped(headPath.node)) {
const names = Object.keys(headPath.getBindingIdentifiers());
const headScope = headPath.scope;

for (const name of names) {
for (let name of names) {
if (bodyScope?.hasOwnBinding(name)) continue; // shadowed

let binding = headPath.scope.getOwnBinding(name);
let binding = headScope.getOwnBinding(name);
if (!binding) {
headPath.scope.crawl();
binding = headPath.scope.getOwnBinding(name);
headScope.crawl();
binding = headScope.getOwnBinding(name);
}
const { usages, capturedInClosure, hasConstantViolations } =
getUsageInBody(binding, path);

if (capturedInClosure) {
markNeedsBodyWrap();
captured.push(name);
} else if (headScope.parent.hasBinding(name)) {
// If the binding is not captured, there is no need
// of adding it to the closure param. However, rename
// it if it shadows an outer binding, because the
// closure will be moved to an outer level.
const newName = headScope.generateUid(name);
headPath.scope.rename(name, newName);
name = newName;
}

if (isForStatement && hasConstantViolations) {
updatedBindingsUsages.set(name, usages);
}
Expand Down
14 changes: 6 additions & 8 deletions packages/babel-plugin-transform-block-scoping/src/loop.ts
Expand Up @@ -177,23 +177,21 @@ export function wrapLoopBody(
const callArgs = [];
const closureParams = [];
const updater = [];
for (const name of captured) {
for (const [name, updatedUsage] of updatedBindingsUsages) {
callArgs.push(t.identifier(name));

const updatedUsage = updatedBindingsUsages.get(name);
if (!updatedUsage) {
// Not updated, re-use the same name
closureParams.push(t.identifier(name));
continue;
}

const innerName = loopPath.scope.generateUid(name);
closureParams.push(t.identifier(innerName));
updater.push(
t.assignmentExpression("=", t.identifier(name), t.identifier(innerName)),
);
for (const path of updatedUsage) path.replaceWith(t.identifier(innerName));
}
for (const name of captured) {
if (updatedBindingsUsages.has(name)) continue; // already injected
callArgs.push(t.identifier(name));
closureParams.push(t.identifier(name));
}

const id = loopPath.scope.generateUid("loop");
const fn = t.functionExpression(
Expand Down
@@ -0,0 +1,9 @@
let res = [];

for (let i = 1; i < 6; i++) {
let y = i;
res.push((() => y)());
i++;
}

expect(res).toEqual([1, 3, 5]);
@@ -0,0 +1,5 @@
for (let i = 4; i < 6; i++) {
let y = i;
() => y;
i++;
}
@@ -0,0 +1,11 @@
var _loop = function (_i) {
var y = _i;
(function () {
return y;
});
_i++;
i = _i;
};
for (var i = 4; i < 6; i++) {
_loop(i);
}
@@ -0,0 +1,8 @@
let res = [];

let i = 0;
for (let i = 4; i < 6; i++) {
res.push((() => i)());
}

expect(res).toEqual([4, 5]);
@@ -0,0 +1,4 @@
let i;
for (let i = 4; i < 6; i++) {
() => i;
}
@@ -0,0 +1,9 @@
var i;
var _loop = function (i) {
(function () {
return i;
});
};
for (var _i = 4; _i < 6; _i++) {
_loop(_i);
}
@@ -0,0 +1,10 @@
let res = [];

let i = 0;
for (let i = 1; i < 6; i++) {
let y = i;
res.push((() => y)());
i++;
}

expect(res).toEqual([1, 3, 5]);
@@ -0,0 +1,6 @@
let i;
for (let i = 4; i < 6; i++) {
let y = i;
() => y;
i++;
}
@@ -0,0 +1,12 @@
var i;
var _loop = function (_i2) {
var y = _i2;
(function () {
return y;
});
_i2++;
_i = _i2;
};
for (var _i = 4; _i < 6; _i++) {
_loop(_i);
}
@@ -0,0 +1,13 @@
let res = [];

for (let i = 1; i < 3; i++) {
let x = i;
res.push((() => x)());
}

for (let i = 4; i < 6; i++) {
let y = i;
res.push((() => y)());
}

expect(res).toEqual([1, 2, 4, 5]);
@@ -0,0 +1,9 @@
for (let i = 1; i < 3; i++) {
let x = i;
() => x;
}

for (let i = 4; i < 6; i++) {
let y = i;
() => y;
}
@@ -0,0 +1,18 @@
var _loop = function () {
var x = i;
(function () {
return x;
});
};
for (var i = 1; i < 3; i++) {
_loop();
}
var _loop2 = function () {
var y = _i;
(function () {
return y;
});
};
for (var _i = 4; _i < 6; _i++) {
_loop2();
}
@@ -0,0 +1,9 @@
let res = [];

let i = 0;
for (let i = 4; i < 6; i++) {
let y = i;
res.push((() => y)());
}

expect(res).toEqual([4, 5]);
@@ -0,0 +1,5 @@
let i;
for (let i = 4; i < 6; i++) {
let y = i;
() => y;
}
@@ -0,0 +1,10 @@
var i;
var _loop = function () {
var y = _i;
(function () {
return y;
});
};
for (var _i = 4; _i < 6; _i++) {
_loop();
}