From 1f92e5a15ca89b7c756a0909262caabf40074506 Mon Sep 17 00:00:00 2001 From: Henry Zhu Date: Fri, 4 Mar 2016 18:02:21 -0500 Subject: [PATCH 1/4] Failing test for nested async with const --- .../fixtures/transformation/misc/regression-2892/actual.js | 6 ++++++ .../transformation/misc/regression-2892/options.json | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/babel-core/test/fixtures/transformation/misc/regression-2892/actual.js b/packages/babel-core/test/fixtures/transformation/misc/regression-2892/actual.js index b49ec4751c..712259dbc9 100644 --- a/packages/babel-core/test/fixtures/transformation/misc/regression-2892/actual.js +++ b/packages/babel-core/test/fixtures/transformation/misc/regression-2892/actual.js @@ -3,3 +3,9 @@ export default class Foo { const baz = 0; } } + +async function foo() { + async function bar() { + const baz = {}; + } +} diff --git a/packages/babel-core/test/fixtures/transformation/misc/regression-2892/options.json b/packages/babel-core/test/fixtures/transformation/misc/regression-2892/options.json index 070c7a47ec..5df260afd8 100644 --- a/packages/babel-core/test/fixtures/transformation/misc/regression-2892/options.json +++ b/packages/babel-core/test/fixtures/transformation/misc/regression-2892/options.json @@ -1,3 +1,4 @@ - { - "plugins": ["external-helpers", "transform-async-to-generator", "transform-regenerator"] +{ + "plugins": ["transform-async-to-generator"], + "presets": ["es2015"] } From 77c7cc5363044f899301b50cd29de67197d06fd2 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Mon, 7 Mar 2016 18:19:10 -0800 Subject: [PATCH 2/4] Rework scope info updating in block-scoping transform I previously tried an approach to scope bindings from var to scope but it didn't catch all cases. This is evident in this bug: https://phabricator.babeljs.io/T2892 Where even after transforming a const to a var we still get an error that it's read-only. This approach will go through and delete every existing let and const binding and creates a new one with the kind "var" --- .../misc/regression-2892/expected.js | 97 ++++++++++++++++--- .../src/index.js | 43 ++++---- packages/babel-traverse/src/scope/binding.js | 3 + 3 files changed, 108 insertions(+), 35 deletions(-) diff --git a/packages/babel-core/test/fixtures/transformation/misc/regression-2892/expected.js b/packages/babel-core/test/fixtures/transformation/misc/regression-2892/expected.js index ca72853118..18b227569b 100644 --- a/packages/babel-core/test/fixtures/transformation/misc/regression-2892/expected.js +++ b/packages/babel-core/test/fixtures/transformation/misc/regression-2892/expected.js @@ -1,19 +1,88 @@ -export default class Foo { - bar() { - var _this = this; +"use strict"; - return babelHelpers.asyncToGenerator(regeneratorRuntime.mark(function _callee() { - var baz; - return regeneratorRuntime.wrap(function _callee$(_context) { - while (1) switch (_context.prev = _context.next) { +Object.defineProperty(exports, "__esModule", { + value: true +}); + +var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }(); + +var foo = function () { + var ref = _asyncToGenerator(regeneratorRuntime.mark(function _callee3() { + var bar = function () { + var ref = _asyncToGenerator(regeneratorRuntime.mark(function _callee2() { + var baz; + return regeneratorRuntime.wrap(function _callee2$(_context2) { + while (1) { + switch (_context2.prev = _context2.next) { + case 0: + baz = {}; + + case 1: + case "end": + return _context2.stop(); + } + } + }, _callee2, this); + })); + + return function bar() { + return ref.apply(this, arguments); + }; + }(); + + return regeneratorRuntime.wrap(function _callee3$(_context3) { + while (1) { + switch (_context3.prev = _context3.next) { case 0: - baz = 0; - - case 1: case "end": - return _context.stop(); + return _context3.stop(); } - }, _callee, _this); - }))(); + } + }, _callee3, this); + })); + + return function foo() { + return ref.apply(this, arguments); + }; +}(); + +function _asyncToGenerator(fn) { return function () { var gen = fn.apply(this, arguments); return new Promise(function (resolve, reject) { function step(key, arg) { try { var info = gen[key](arg); var value = info.value; } catch (error) { reject(error); return; } if (info.done) { resolve(value); } else { return Promise.resolve(value).then(function (value) { return step("next", value); }, function (err) { return step("throw", err); }); } } return step("next"); }); }; } + +function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } } + +var Foo = function () { + function Foo() { + _classCallCheck(this, Foo); } -} + + _createClass(Foo, [{ + key: "bar", + value: function () { + var ref = _asyncToGenerator(regeneratorRuntime.mark(function _callee() { + var baz; + return regeneratorRuntime.wrap(function _callee$(_context) { + while (1) { + switch (_context.prev = _context.next) { + case 0: + baz = 0; + + case 1: + case "end": + return _context.stop(); + } + } + }, _callee, this); + })); + + function bar() { + return ref.apply(this, arguments); + } + + return bar; + }() + }]); + + return Foo; +}(); + +exports.default = Foo; diff --git a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js index 079a126478..43e7b75eb0 100644 --- a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js +++ b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js @@ -88,10 +88,6 @@ function convertBlockScopedToVar(path, parent, scope, moveBindingsToParent = fal // Move bindings from current block scope to function scope. if (moveBindingsToParent) { const parentScope = scope.getFunctionParent(); - if (parentScope === scope) { - return; - } - const ids = path.getBindingIdentifiers(); for (let name in ids) { scope.removeOwnBinding(name); @@ -302,7 +298,6 @@ class BlockScoping { this.outsideLetReferences = Object.create(null); this.hasLetReferences = false; this.letReferences = Object.create(null); - this.letReferencesDeclars = []; this.body = []; if (loopPath) { @@ -325,7 +320,10 @@ class BlockScoping { let needsClosure = this.getLetReferences(); // this is a block within a `Function/Program` so we can safely leave it be - if (t.isFunction(this.parent) || t.isProgram(this.block)) return; + if (t.isFunction(this.parent) || t.isProgram(this.block)) { + this.updateScopeInfo(); + return; + } // we can skip everything if (!this.hasLetReferences) return; @@ -336,18 +334,34 @@ class BlockScoping { this.remap(); } + this.updateScopeInfo(); + if (this.loopLabel && !t.isLabeledStatement(this.loopParent)) { return t.labeledStatement(this.loopLabel, this.loop); } } + updateScopeInfo() { + let scope = this.scope; + let parentScope = scope.getFunctionParent(); + let letRefs = this.letReferences; + + const i = 0; + for (let key in letRefs) { + let ref = letRefs[key]; + const binding = scope.getBinding(ref.name); + if (!binding) continue; + if (binding.kind === 'let' || binding.kind === 'const') { + scope.removeOwnBinding(ref.name); + parentScope.registerBinding("var", binding.path); + } + } + } remap() { let hasRemaps = false; let letRefs = this.letReferences; let scope = this.scope; - const parentScope = scope.getFunctionParent(); - // alright, so since we aren't wrapping this block in a closure // we have to check if any of our let variables collide with // those in upper scopes and then if they do, generate a uid @@ -370,18 +384,6 @@ class BlockScoping { uid: uid }; } - - // Remove binding from block scope so it's moved to function scope. - if (parentScope !== scope) { - scope.removeOwnBinding(ref.name); - } - } - - // Move bindings to parent scope. - if (parentScope !== scope) { - for (let declar of (this.letReferencesDeclars: Array)) { - parentScope.registerBinding("var", declar); - } } if (!hasRemaps) return; @@ -535,7 +537,6 @@ class BlockScoping { let declarPath = this.blockPath.get("body")[i]; if (isBlockScoped(declar)) { convertBlockScopedToVar(declarPath, block, this.scope); - this.letReferencesDeclars.push(declarPath); } declarators = declarators.concat(declar.declarations || declar); } diff --git a/packages/babel-traverse/src/scope/binding.js b/packages/babel-traverse/src/scope/binding.js index 57decb22a9..1578de17d4 100644 --- a/packages/babel-traverse/src/scope/binding.js +++ b/packages/babel-traverse/src/scope/binding.js @@ -71,6 +71,9 @@ export default class Binding { reassign(path: Object) { this.constant = false; + if (this.constantViolations.includes(path)) { + return; + } this.constantViolations.push(path); } From 3bebc3a7ca36e4079f80edeeba867d8a9a27a374 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Mon, 7 Mar 2016 18:26:51 -0800 Subject: [PATCH 3/4] lint --- .../babel-plugin-transform-es2015-block-scoping/src/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js index 43e7b75eb0..727a66e3b1 100644 --- a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js +++ b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js @@ -346,12 +346,11 @@ class BlockScoping { let parentScope = scope.getFunctionParent(); let letRefs = this.letReferences; - const i = 0; for (let key in letRefs) { let ref = letRefs[key]; const binding = scope.getBinding(ref.name); if (!binding) continue; - if (binding.kind === 'let' || binding.kind === 'const') { + if (binding.kind === "let" || binding.kind === "const") { scope.removeOwnBinding(ref.name); parentScope.registerBinding("var", binding.path); } From 0200542e820be1f57c44bbe19792fea4f4e6eca6 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Tue, 8 Mar 2016 00:33:37 -0800 Subject: [PATCH 4/4] don't use Array.includes --- packages/babel-traverse/src/scope/binding.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-traverse/src/scope/binding.js b/packages/babel-traverse/src/scope/binding.js index 1578de17d4..04e9a14eda 100644 --- a/packages/babel-traverse/src/scope/binding.js +++ b/packages/babel-traverse/src/scope/binding.js @@ -71,7 +71,7 @@ export default class Binding { reassign(path: Object) { this.constant = false; - if (this.constantViolations.includes(path)) { + if (this.constantViolations.indexOf(path) !== -1) { return; } this.constantViolations.push(path);