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

Avoid adding unnecessary closure for block scoping #5246

Merged
merged 1 commit into from Feb 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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
@@ -1,6 +1,6 @@
function wrapper(fn) {
return (...args) => {
if (someCondition) {
while (someCondition) {
const val = fn(...args);
return val.test(() => {
console.log(val);
Expand Down
Expand Up @@ -2,17 +2,19 @@ function wrapper(fn) {
return function () {
var _arguments = arguments;

if (someCondition) {
var _ret = function () {
var val = fn(..._arguments);
return {
v: val.test(function () {
console.log(val);
})
};
}();
var _loop = function () {
var val = fn(..._arguments);
return {
v: val.test(function () {
console.log(val);
})
};
};

while (someCondition) {
var _ret = _loop();

if (typeof _ret === "object") return _ret.v;
}
};
}
}
27 changes: 25 additions & 2 deletions packages/babel-plugin-transform-es2015-block-scoping/src/index.js
Expand Up @@ -112,8 +112,21 @@ function isVar(node) {
}

const letReferenceBlockVisitor = traverse.visitors.merge([{
Loop: {
enter(path, state) {
state.loopDepth++;
},
exit(path, state) {
state.loopDepth--;
},
},
Function(path, state) {
path.traverse(letReferenceFunctionVisitor, state);
// References to block-scoped variables only require added closures if it's
// possible for the code to run more than once -- otherwise it is safe to
// simply rename the variables.
if (state.loopDepth > 0) {
path.traverse(letReferenceFunctionVisitor, state);
}
return path.skip();
}
}, tdzVisitor]);
Expand Down Expand Up @@ -549,9 +562,19 @@ class BlockScoping {
const state = {
letReferences: this.letReferences,
closurify: false,
file: this.file
file: this.file,
loopDepth: 0,
};

const loopOrFunctionParent = this.blockPath.find(
(path) => path.isLoop() || path.isFunction()
);
if (loopOrFunctionParent && loopOrFunctionParent.isLoop()) {
// There is a loop ancestor closer than the closest function, so we
// consider ourselves to be in a loop.
state.loopDepth++;
}

// traverse through this block, stopping on functions and checking if they
// contain any local let references
this.blockPath.traverse(letReferenceBlockVisitor, state);
Expand Down
@@ -1,11 +1,9 @@
if (true) {
var x;
var foo = function () {};

(function () {
function foo() {}
function bar() {
return foo;
}
for (x in {}) {}
})();
var bar = function () {
return foo;
};

for (var x in {}) {}
}
@@ -0,0 +1,34 @@
function foo() {
const x = 5;
console.log(x);

{
const x = 7;
setTimeout(() => x, 0);
}
}

function bar() {
const x = 5;
console.log(x);

for (let i = 0; i < 7; i++) {
{
const x = i;
setTimeout(() => x, 0);
}
}
}

function baz() {
const x = 5;
console.log(x);

for (let i = 0; i < 7; i++) {
var qux = function qux(y) {
const x = y;
setTimeout(() => x, 0);
};
qux(i);
}
}
@@ -0,0 +1,42 @@
function foo() {
var x = 5;
console.log(x);

{
var _x = 7;
setTimeout(function () {
return _x;
}, 0);
}
}

function bar() {
var x = 5;
console.log(x);

for (var i = 0; i < 7; i++) {
{
(function () {
var x = i;
setTimeout(function () {
return x;
}, 0);
})();
}
}
}

function baz() {
var x = 5;
console.log(x);

for (var i = 0; i < 7; i++) {
var qux = function qux(y) {
var x = y;
setTimeout(function () {
return x;
}, 0);
};
qux(i);
}
}
@@ -0,0 +1,11 @@
var f1, f2;
{
let z = 'z1 value';
f1 = function() { return z; };
}
{
let z = 'z2 value';
f2 = function() { return z; };
}
f1();
f2();
@@ -0,0 +1,15 @@
var f1, f2;
{
var z = 'z1 value';
f1 = function () {
return z;
};
}
{
var _z = 'z2 value';
f2 = function () {
return _z;
};
}
f1();
f2();
@@ -1,16 +1,18 @@
function foo() {
switch (2) {
case 0: {
if (true) {
return;
}
while (true) {
switch (2) {
case 0: {
if (true) {
return;
}

const stuff = new Map();
const data = 0;
stuff.forEach(() => {
const d = data;
});
break;
const stuff = new Map();
const data = 0;
stuff.forEach(() => {
const d = data;
});
break;
}
}
}
}
@@ -1,29 +1,31 @@
function foo() {
switch (2) {
case 0:
{
var _ret = function () {
if (true) {
return {
v: void 0
};
}
while (true) {
switch (2) {
case 0:
{
var _ret = function () {
if (true) {
return {
v: void 0
};
}

var stuff = new Map();
var data = 0;
stuff.forEach(function () {
var d = data;
});
return "break";
}();
var stuff = new Map();
var data = 0;
stuff.forEach(function () {
var d = data;
});
return "break";
}();

switch (_ret) {
case "break":
break;
switch (_ret) {
case "break":
break;

default:
if (typeof _ret === "object") return _ret.v;
default:
if (typeof _ret === "object") return _ret.v;
}
}
}
}
}
}
@@ -1,10 +1,12 @@
function fn() {
switch (true) {
default:
let foo = 4;
if (true) {
let bar = () => foo;
console.log(bar());
while (true) {
switch (true) {
default:
let foo = 4;
if (true) {
let bar = () => foo;
console.log(bar());
}
}
}
}
@@ -1,14 +1,16 @@
function fn() {
(function () {
switch (true) {
default:
var foo = 4;
if (true) {
var bar = function () {
return foo;
};
console.log(bar());
}
}
})();
while (true) {
(function () {
switch (true) {
default:
var foo = 4;
if (true) {
var bar = function () {
return foo;
};
console.log(bar());
}
}
})();
}
}
@@ -0,0 +1,6 @@
for (var i = 0; i < 5; i++) {
var l = i;
setTimeout(function () {
console.log(l);
}, 1);
}

This file was deleted.

This file was deleted.

@@ -1,17 +1,11 @@
function render(flag) {
if (flag) {
var _ret = function () {
var bar = "bar";
var bar = "bar";

[].map(() => bar);
[].map(() => bar);

return {
v: <foo bar={bar} />
};
}();

if (typeof _ret === "object") return _ret.v;
return <foo bar={bar} />;
}

return null;
}
}