From e11b943514fd848a13b6dd0668907a46d4723a38 Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Sun, 22 Feb 2015 20:54:47 +1100 Subject: [PATCH] revert to old named function wrapper - fixes #861 --- .../transformation/helpers/name-method.js | 95 ++++++++++++++----- .../transformers/es6/block-scoping.js | 2 +- .../transformers/spec/function-name.js | 95 +------------------ .../method-self-reference/expected.js | 54 +++++++---- 4 files changed, 111 insertions(+), 135 deletions(-) diff --git a/lib/babel/transformation/helpers/name-method.js b/lib/babel/transformation/helpers/name-method.js index 05cbcf1764..5b02942ef0 100644 --- a/lib/babel/transformation/helpers/name-method.js +++ b/lib/babel/transformation/helpers/name-method.js @@ -5,15 +5,13 @@ var t = require("../../types"); var visitor = { enter: function (node, parent, scope, state) { - // check if this node is an identifier that matches the same as our function id - if (!t.isIdentifier(node, { name: state.id })) return; - - // check if this node is the one referenced - if (!t.isReferenced(node, parent)) return; + // check if this node is a referenced identifier that matches the same as our + // function id + if (!t.isReferencedIdentifier(node, parent, { name: state.name })) return; // check that we don't have a local variable declared as that removes the need // for the wrapper - var localDeclar = scope.getBindingIdentifier(state.id); + var localDeclar = scope.getBindingIdentifier(state.name); if (localDeclar !== state.outerDeclar) return; state.selfReference = true; @@ -21,33 +19,80 @@ var visitor = { } }; +var wrap = function (method, id, scope) { + var templateName = "property-method-assignment-wrapper"; + if (method.generator) templateName += "-generator"; + return util.template(templateName, { + FUNCTION: method, + FUNCTION_ID: id, + FUNCTION_KEY: scope.generateUidIdentifier(id.name), + WRAPPER_KEY: scope.generateUidIdentifier(id.name + "Wrapper") + }); +}; + +var visit = function (node, name, scope) { + var state = { + name: name, + selfReference: false, + outerDeclar: scope.getBindingIdentifier(name), + }; + + if (doesntHaveLocal(name, scope)) { + scope.traverse(node, visitor, state); + } + + return state; +}; + +var doesntHaveLocal = function (name, scope) { + var bindingInfo = scope.getOwnBindingInfo(name); + return !bindingInfo || bindingInfo.kind !== "param"; +}; + exports.property = function (node, file, scope) { var key = t.toComputedKey(node, node.key); if (!t.isLiteral(key)) return node; // we can't set a function id with this - var id = t.toIdentifier(key.value); - key = t.identifier(id); - - var state = { - id: id, - selfReference: false, - outerDeclar: scope.getBindingIdentifier(id), - }; - - scope.traverse(node, visitor, state); + var name = t.toIdentifier(key.value); + var id = t.identifier(name); var method = node.value; + var state = visit(method, name, scope); if (state.selfReference) { - var templateName = "property-method-assignment-wrapper"; - if (method.generator) templateName += "-generator"; - node.value = util.template(templateName, { - FUNCTION: method, - FUNCTION_ID: key, - FUNCTION_KEY: scope.generateUidIdentifier(id), - WRAPPER_KEY: scope.generateUidIdentifier(id + "Wrapper") - }); + node.value = wrap(method, id, scope); } else { - method.id = key; + method.id = id; + } +}; + +exports.bare = function (node, parent, scope) { + // has an `id` so we don't need to infer one + if (node.id) return; + + var id; + if (t.isProperty(parent) && parent.kind === "init" && !parent.computed) { + // { foo: function () {} }; + id = parent.key; + } else if (t.isVariableDeclarator(parent)) { + // var foo = function () {}; + id = parent.id; + } else { + return; + } + + if (!t.isIdentifier(id)) return; + + var name = t.toIdentifier(id.name); + id = t.identifier(name); + + // + + var state = visit(node, name, scope); + + if (state.selfReference) { + return wrap(node, id, scope); + } else { + node.id = id; } }; diff --git a/lib/babel/transformation/transformers/es6/block-scoping.js b/lib/babel/transformation/transformers/es6/block-scoping.js index bf486fb2b1..984e5585f3 100644 --- a/lib/babel/transformation/transformers/es6/block-scoping.js +++ b/lib/babel/transformation/transformers/es6/block-scoping.js @@ -30,7 +30,7 @@ var isLet = function (node, parent) { }; var isLetInitable = function (node, parent) { - return !t.isFor(parent) || t.isFor(parent) && parent.left !== node; + return !t.isFor(parent) || !t.isFor(parent, { left: node }); }; var isVar = function (node, parent) { diff --git a/lib/babel/transformation/transformers/spec/function-name.js b/lib/babel/transformation/transformers/spec/function-name.js index d52201470b..0a49b0a38b 100644 --- a/lib/babel/transformation/transformers/spec/function-name.js +++ b/lib/babel/transformation/transformers/spec/function-name.js @@ -1,95 +1,8 @@ "use strict"; -var t = require("../../../types"); -var util = require("../../../util"); - -var propertyFunctionVisitor = { - enter: function (node, parent, scope, state) { - if (t.isReferencedIdentifier(node, parent, { name: state.name }) && scope.getBindingIdentifier(node.name) === state.binding) { - return state.getOuter(); - } - } -}; +var nameMethod = require("../../helpers/name-method"); +var util = require("../../../util"); +var t = require("../../../types"); //exports.ArrowFunctionExpression = -exports.FunctionExpression = function (node, parent, scope) { - // has an `id` so we don't need to infer one - if (node.id) return; - - var id; - if (t.isProperty(parent) && parent.kind === "init" && !parent.computed) { - // { foo: function () {} }; - id = parent.key; - } else if (t.isVariableDeclarator(parent)) { - // var foo = function () {}; - id = parent.id; - } else { - return; - } - - if (!t.isIdentifier(id)) return; - - var name = t.toIdentifier(id.name); - id = t.identifier(name); - - // check to see if we have a local binding of the id we're setting inside of - // the function, this is important as there are caveats associated - - var bindingInfo = scope.getOwnBindingInfo(name); - - if (bindingInfo) { - if (bindingInfo.type === "param") { - // safari will blow up in strict mode with code like: - // - // var t = function t(t) {}; - // - // with the error: - // - // Cannot declare a parameter named 't' as it shadows the name of a - // strict mode function. - // - // this isn't to the spec and they've invented this behaviour which is - // **extremely** annoying so we avoid setting the name if it has a param - // with the same id - } else { - // otherwise it's defined somewhere in scope like: - // - // var t = function () { - // var t = 2; - // }; - // - // so we can safely just set the id and move along as it shadows the - // bound function id - node.id = id; - } - - return; - } - - // - - var binding = scope.getBindingIdentifier(name); - var outerId; - - scope.traverse(node, propertyFunctionVisitor, { - name: name, - binding: binding, - - getOuter: function () { - return t.callExpression( - outerId || (outerId = scope.generateUidIdentifier("getOuter")), - [] - ); - } - }); - - node.id = id; - - if (outerId) { - return util.template("named-func", { - GET_OUTER_ID: outerId, - FUNCTION: node, - ID: id - }); - } -}; +exports.FunctionExpression = nameMethod.bare; diff --git a/test/fixtures/transformation/es6-properties.shorthand/method-self-reference/expected.js b/test/fixtures/transformation/es6-properties.shorthand/method-self-reference/expected.js index 556770fa0e..626881c7c8 100644 --- a/test/fixtures/transformation/es6-properties.shorthand/method-self-reference/expected.js +++ b/test/fixtures/transformation/es6-properties.shorthand/method-self-reference/expected.js @@ -1,33 +1,51 @@ "use strict"; var bar = { - foo: (function () { - function _getOuter() { - return foo; - } - - return function foo() { - console.log(_getOuter()); + foo: (function (_foo) { + var _fooWrapper = function foo() { + return _foo.apply(this, arguments); }; - })() + + _fooWrapper.toString = function () { + return _foo.toString(); + }; + + return _fooWrapper; + })(function () { + console.log(foo); + }) }; var bar = { - foo: function foo() { + foo: (function (_foo) { + var _fooWrapper = function foo() { + return _foo.apply(this, arguments); + }; + + _fooWrapper.toString = function () { + return _foo.toString(); + }; + + return _fooWrapper; + })(function () { var foo = 41; console.log(foo); - } + }) }; var foobar = 123; var foobar2 = { - foobar: (function () { - function _getOuter() { - return foobar; - } - - return function foobar() { - console.log(_getOuter()); + foobar: (function (_foobar) { + var _foobarWrapper = function foobar() { + return _foobar.apply(this, arguments); }; - })() + + _foobarWrapper.toString = function () { + return _foobar.toString(); + }; + + return _foobarWrapper; + })(function () { + console.log(foobar); + }) };