From 95882d4e5a8446d70a831dcc9527704cbc1a739b Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Mon, 1 May 2017 16:17:55 -0700 Subject: [PATCH 1/5] Rewrite param processing to be more clearly defined. --- .../destructuring/parameters/expected.js | 16 +- .../src/default.js | 185 ---------------- .../src/destructuring.js | 29 --- .../src/index.js | 41 ++-- .../src/params.js | 115 ++++++++++ .../src/rest.js | 200 +++++++++--------- .../parameters/default-rest-mix/actual.js | 8 + .../parameters/default-rest-mix/expected.js | 11 + .../parameters/destructuring-rest/expected.js | 6 +- 9 files changed, 260 insertions(+), 351 deletions(-) delete mode 100644 packages/babel-plugin-transform-es2015-parameters/src/default.js delete mode 100644 packages/babel-plugin-transform-es2015-parameters/src/destructuring.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/src/params.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/actual.js create mode 100644 packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/expected.js diff --git a/packages/babel-plugin-transform-es2015-destructuring/test/fixtures/destructuring/parameters/expected.js b/packages/babel-plugin-transform-es2015-destructuring/test/fixtures/destructuring/parameters/expected.js index 0b999f630b..12bb63d7b5 100644 --- a/packages/babel-plugin-transform-es2015-destructuring/test/fixtures/destructuring/parameters/expected.js +++ b/packages/babel-plugin-transform-es2015-destructuring/test/fixtures/destructuring/parameters/expected.js @@ -21,15 +21,15 @@ console.log(unpackObject({ })); var unpackArray = function (_ref3, _ref4) { - var _ref6 = babelHelpers.slicedToArray(_ref3, 3), - a = _ref6[0], - b = _ref6[1], - c = _ref6[2]; + var _ref5 = babelHelpers.slicedToArray(_ref3, 3), + a = _ref5[0], + b = _ref5[1], + c = _ref5[2]; - var _ref5 = babelHelpers.slicedToArray(_ref4, 3), - x = _ref5[0], - y = _ref5[1], - z = _ref5[2]; + var _ref6 = babelHelpers.slicedToArray(_ref4, 3), + x = _ref6[0], + y = _ref6[1], + z = _ref6[2]; return a + b + c; }; diff --git a/packages/babel-plugin-transform-es2015-parameters/src/default.js b/packages/babel-plugin-transform-es2015-parameters/src/default.js deleted file mode 100644 index f3b1875e76..0000000000 --- a/packages/babel-plugin-transform-es2015-parameters/src/default.js +++ /dev/null @@ -1,185 +0,0 @@ -import getFunctionArity from "babel-helper-get-function-arity"; -import callDelegate from "babel-helper-call-delegate"; -import template from "babel-template"; -import * as t from "babel-types"; - -const buildDefaultParam = template(` - let VARIABLE_NAME = - ARGUMENTS.length > ARGUMENT_KEY && ARGUMENTS[ARGUMENT_KEY] !== undefined ? - ARGUMENTS[ARGUMENT_KEY] - : - DEFAULT_VALUE; -`); - -const buildLooseDefaultParam = template(` - if (ASSIGMENT_IDENTIFIER === UNDEFINED) { - ASSIGMENT_IDENTIFIER = DEFAULT_VALUE; - } -`); - -const buildLooseDestructuredDefaultParam = template(` - let ASSIGMENT_IDENTIFIER = PARAMETER_NAME === UNDEFINED ? DEFAULT_VALUE : PARAMETER_NAME ; -`); - -const buildCutOff = template(` - let $0 = $1[$2]; -`); - -function hasDefaults(node) { - for (const param of (node.params: Array)) { - if (!t.isIdentifier(param)) return true; - } - return false; -} - -function isSafeBinding(scope, node) { - if (!scope.hasOwnBinding(node.name)) return true; - const { kind } = scope.getOwnBinding(node.name); - return kind === "param" || kind === "local"; -} - -const iifeVisitor = { - ReferencedIdentifier(path, state) { - const { scope, node } = path; - if (node.name === "eval" || !isSafeBinding(scope, node)) { - state.iife = true; - path.stop(); - } - }, - - Scope(path) { - // different bindings - path.skip(); - }, -}; - -export const visitor = { - Function(path) { - const { node, scope } = path; - if (!hasDefaults(node)) return; - - // ensure it's a block, useful for arrow functions - path.ensureBlock(); - - const params = path.get("params"); - - if (this.opts.loose) { - const body = []; - for (let i = 0; i < params.length; ++i) { - const param = params[i]; - if (param.isAssignmentPattern()) { - const left = param.get("left"); - const right = param.get("right"); - - const undefinedNode = scope.buildUndefinedNode(); - - if (left.isIdentifier()) { - body.push( - buildLooseDefaultParam({ - ASSIGMENT_IDENTIFIER: left.node, - DEFAULT_VALUE: right.node, - UNDEFINED: undefinedNode, - }), - ); - param.replaceWith(left.node); - } else if (left.isObjectPattern() || left.isArrayPattern()) { - const paramName = scope.generateUidIdentifier(); - body.push( - buildLooseDestructuredDefaultParam({ - ASSIGMENT_IDENTIFIER: left.node, - DEFAULT_VALUE: right.node, - PARAMETER_NAME: paramName, - UNDEFINED: undefinedNode, - }), - ); - param.replaceWith(paramName); - } - } - } - path.get("body").unshiftContainer("body", body); - return; - } - - const state = { - iife: false, - scope: scope, - }; - - const body = []; - - // - const argsIdentifier = t.identifier("arguments"); - - // push a default parameter definition - function pushDefNode(left, right, i) { - const defNode = buildDefaultParam({ - VARIABLE_NAME: left, - DEFAULT_VALUE: right, - ARGUMENT_KEY: t.numericLiteral(i), - ARGUMENTS: argsIdentifier, - }); - defNode._blockHoist = node.params.length - i; - body.push(defNode); - } - - // - const lastNonDefaultParam = getFunctionArity(node); - - // - for (let i = 0; i < params.length; i++) { - const param = params[i]; - - if (!param.isAssignmentPattern()) { - if (!state.iife && !param.isIdentifier()) { - param.traverse(iifeVisitor, state); - } - - continue; - } - - const left = param.get("left"); - const right = param.get("right"); - - // - if (i >= lastNonDefaultParam || left.isPattern()) { - const placeholder = scope.generateUidIdentifier("x"); - placeholder._isDefaultPlaceholder = true; - node.params[i] = placeholder; - } else { - node.params[i] = left.node; - } - - // - if (!state.iife) { - if (right.isIdentifier() && !isSafeBinding(scope, right.node)) { - // the right hand side references a parameter - state.iife = true; - } else { - right.traverse(iifeVisitor, state); - } - } - - pushDefNode(left.node, right.node, i); - } - - // add declarations for trailing parameters - for (let i = lastNonDefaultParam + 1; i < node.params.length; i++) { - const param = node.params[i]; - if (param._isDefaultPlaceholder) continue; - - const declar = buildCutOff(param, argsIdentifier, t.numericLiteral(i)); - declar._blockHoist = node.params.length - i; - body.push(declar); - } - - // we need to cut off all trailing parameters - node.params = node.params.slice(0, lastNonDefaultParam); - - if (state.iife) { - body.push(callDelegate(path, scope)); - path.set("body", t.blockStatement(body)); - } else { - path.get("body").unshiftContainer("body", body); - } - }, -}; diff --git a/packages/babel-plugin-transform-es2015-parameters/src/destructuring.js b/packages/babel-plugin-transform-es2015-parameters/src/destructuring.js deleted file mode 100644 index c40db2dac9..0000000000 --- a/packages/babel-plugin-transform-es2015-parameters/src/destructuring.js +++ /dev/null @@ -1,29 +0,0 @@ -import * as t from "babel-types"; - -export const visitor = { - Function(path) { - const params: Array = path.get("params"); - - // If there's a rest param, no need to loop through it. Also, we need to - // hoist one more level to get `declar` at the right spot. - const hoistTweak = t.isRestElement(params[params.length - 1]) ? 1 : 0; - const outputParamsLength = params.length - hoistTweak; - - for (let i = 0; i < outputParamsLength; i++) { - const param = params[i]; - if (param.isArrayPattern() || param.isObjectPattern()) { - const uid = path.scope.generateUidIdentifier("ref"); - - const declar = t.variableDeclaration("let", [ - t.variableDeclarator(param.node, uid), - ]); - declar._blockHoist = outputParamsLength - i; - - path.ensureBlock(); - path.get("body").unshiftContainer("body", declar); - - param.replaceWith(uid); - } - } - }, -}; diff --git a/packages/babel-plugin-transform-es2015-parameters/src/index.js b/packages/babel-plugin-transform-es2015-parameters/src/index.js index e9b4117ccd..f3ffba2755 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/index.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/index.js @@ -1,32 +1,23 @@ -import type { NodePath } from "babel-traverse"; -import { visitors } from "babel-traverse"; - -import * as destructuring from "./destructuring"; -import * as def from "./default"; -import * as rest from "./rest"; +import convertFunctionParams from "./params"; +import convertFunctionRest from "./rest"; export default function() { return { - visitor: visitors.merge([ - { - ArrowFunctionExpression(path) { - // In some conversion cases, it may have already been converted to a function while this callback - // was queued up. - if (!path.isArrowFunctionExpression()) return; + visitor: { + Function(path) { + if ( + path.isArrowFunctionExpression() && + path + .get("params") + .some(param => param.isRestElement() || param.isAssignmentPattern()) + ) { + // default/rest visitors require access to `arguments`, so it cannot be an arrow + path.arrowFunctionToExpression(); + } - // default/rest visitors require access to `arguments` - const params: Array = path.get("params"); - for (const param of params) { - if (param.isRestElement() || param.isAssignmentPattern()) { - path.arrowFunctionToExpression(); - break; - } - } - }, + convertFunctionRest(path); + convertFunctionParams(path); }, - destructuring.visitor, - rest.visitor, - def.visitor, - ]), + }, }; } diff --git a/packages/babel-plugin-transform-es2015-parameters/src/params.js b/packages/babel-plugin-transform-es2015-parameters/src/params.js new file mode 100644 index 0000000000..f6cf4b1e31 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/src/params.js @@ -0,0 +1,115 @@ +import callDelegate from "babel-helper-call-delegate"; +import template from "babel-template"; +import * as t from "babel-types"; + +const buildDefaultParam = template(` + let VARIABLE_NAME = + arguments.length > ARGUMENT_KEY && arguments[ARGUMENT_KEY] !== undefined ? + arguments[ARGUMENT_KEY] + : + DEFAULT_VALUE; +`); + +const buildArgumentsAccess = template(` + let $0 = arguments[$1]; +`); + +function isSafeBinding(scope, node) { + if (!scope.hasOwnBinding(node.name)) return true; + const { kind } = scope.getOwnBinding(node.name); + return kind === "param" || kind === "local"; +} + +const iifeVisitor = { + ReferencedIdentifier(path, state) { + const { scope, node } = path; + if (node.name === "eval" || !isSafeBinding(scope, node)) { + state.iife = true; + path.stop(); + } + }, + + Scope(path) { + // different bindings + path.skip(); + }, +}; + +export default function convertFunctionParams(path) { + const { node, scope } = path; + + const state = { + iife: false, + scope: scope, + }; + + const body = []; + + let firstOptionalIndex = null; + + // + const params = path.get("params"); + for (let i = 0; i < params.length; i++) { + const param = params[i]; + + if (param.isAssignmentPattern()) { + if (firstOptionalIndex === null) firstOptionalIndex = i; + + const left = param.get("left"); + const right = param.get("right"); + + // + if (!state.iife) { + if (right.isIdentifier() && !isSafeBinding(scope, right.node)) { + // the right hand side references a parameter + state.iife = true; + } else { + right.traverse(iifeVisitor, state); + } + } + + const defNode = buildDefaultParam({ + VARIABLE_NAME: left.node, + DEFAULT_VALUE: right.node, + ARGUMENT_KEY: t.numericLiteral(i), + }); + defNode._blockHoist = node.params.length - i; + body.push(defNode); + } else if (firstOptionalIndex !== null) { + const defNode = buildArgumentsAccess(param.node, t.numericLiteral(i)); + defNode._blockHoist = node.params.length - i; + body.push(defNode); + } else if (param.isObjectPattern() || param.isArrayPattern()) { + const uid = path.scope.generateUidIdentifier("ref"); + + const defNode = t.variableDeclaration("let", [ + t.variableDeclarator(param.node, uid), + ]); + defNode._blockHoist = node.params.length - i; + body.push(defNode); + + param.replaceWith(uid); + } + + if (!state.iife && !param.isIdentifier()) { + param.traverse(iifeVisitor, state); + } + } + + if (body.length === 0) return; + + // we need to cut off all trailing parameters + if (firstOptionalIndex !== null) { + node.params = node.params.slice(0, firstOptionalIndex); + } + + // ensure it's a block, useful for arrow functions + path.ensureBlock(); + + if (state.iife) { + body.push(callDelegate(path, scope)); + path.set("body", t.blockStatement(body)); + } else { + path.get("body").unshiftContainer("body", body); + } +} diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index e74df03e97..c9f7f2906c 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -220,124 +220,122 @@ function optimiseLengthGetter(path, argsId, offset) { } } -export const visitor = { - Function(path) { - const { node, scope } = path; - if (!hasRest(node)) return; +export default function convertFunctionRest(path) { + const { node, scope } = path; + if (!hasRest(node)) return; - const rest = node.params.pop().argument; + const rest = node.params.pop().argument; - const argsId = t.identifier("arguments"); + const argsId = t.identifier("arguments"); - // check and optimise for extremely common cases - const state = { - references: [], - offset: node.params.length, + // check and optimise for extremely common cases + const state = { + references: [], + offset: node.params.length, - argumentsNode: argsId, - outerBinding: scope.getBindingIdentifier(rest.name), + argumentsNode: argsId, + outerBinding: scope.getBindingIdentifier(rest.name), - // candidate member expressions we could optimise if there are no other references - candidates: [], + // candidate member expressions we could optimise if there are no other references + candidates: [], - // local rest binding name - name: rest.name, + // local rest binding name + name: rest.name, - /* - It may be possible to optimize the output code in certain ways, such as - not generating code to initialize an array (perhaps substituting direct - references to arguments[i] or arguments.length for reads of the - corresponding rest parameter property) or positioning the initialization - code so that it may not have to execute depending on runtime conditions. + /* + It may be possible to optimize the output code in certain ways, such as + not generating code to initialize an array (perhaps substituting direct + references to arguments[i] or arguments.length for reads of the + corresponding rest parameter property) or positioning the initialization + code so that it may not have to execute depending on runtime conditions. - This property tracks eligibility for optimization. "deopted" means give up - and don't perform optimization. For example, when any of rest's elements / - properties is assigned to at the top level, or referenced at all in a - nested function. - */ - deopted: false, - }; + This property tracks eligibility for optimization. "deopted" means give up + and don't perform optimization. For example, when any of rest's elements / + properties is assigned to at the top level, or referenced at all in a + nested function. + */ + deopted: false, + }; - path.traverse(memberExpressionOptimisationVisitor, state); + path.traverse(memberExpressionOptimisationVisitor, state); - // There are only "shorthand" references - if (!state.deopted && !state.references.length) { - for (const { path, cause } of (state.candidates: Array)) { - switch (cause) { - case "indexGetter": - optimiseIndexGetter(path, argsId, state.offset); - break; - case "lengthGetter": - optimiseLengthGetter(path, argsId, state.offset); - break; - default: - path.replaceWith(argsId); - } + // There are only "shorthand" references + if (!state.deopted && !state.references.length) { + for (const { path, cause } of (state.candidates: Array)) { + switch (cause) { + case "indexGetter": + optimiseIndexGetter(path, argsId, state.offset); + break; + case "lengthGetter": + optimiseLengthGetter(path, argsId, state.offset); + break; + default: + path.replaceWith(argsId); } - return; } + return; + } - state.references = state.references.concat( - state.candidates.map(({ path }) => path), + state.references = state.references.concat( + state.candidates.map(({ path }) => path), + ); + + const start = t.numericLiteral(node.params.length); + const key = scope.generateUidIdentifier("key"); + const len = scope.generateUidIdentifier("len"); + + let arrKey = key; + let arrLen = len; + if (node.params.length) { + // this method has additional params, so we need to subtract + // the index of the current argument position from the + // position in the array that we want to populate + arrKey = t.binaryExpression("-", key, start); + + // we need to work out the size of the array that we're + // going to store all the rest parameters + // + // we need to add a check to avoid constructing the array + // with <0 if there are less arguments than params as it'll + // cause an error + arrLen = t.conditionalExpression( + t.binaryExpression(">", len, start), + t.binaryExpression("-", len, start), + t.numericLiteral(0), ); + } - const start = t.numericLiteral(node.params.length); - const key = scope.generateUidIdentifier("key"); - const len = scope.generateUidIdentifier("len"); + const loop = buildRest({ + ARGUMENTS: argsId, + ARRAY_KEY: arrKey, + ARRAY_LEN: arrLen, + START: start, + ARRAY: rest, + KEY: key, + LEN: len, + }); - let arrKey = key; - let arrLen = len; - if (node.params.length) { - // this method has additional params, so we need to subtract - // the index of the current argument position from the - // position in the array that we want to populate - arrKey = t.binaryExpression("-", key, start); + if (state.deopted) { + loop._blockHoist = node.params.length + 1; + node.body.body.unshift(loop); + } else { + // perform allocation at the lowest common ancestor of all references + loop._blockHoist = 1; - // we need to work out the size of the array that we're - // going to store all the rest parameters - // - // we need to add a check to avoid constructing the array - // with <0 if there are less arguments than params as it'll - // cause an error - arrLen = t.conditionalExpression( - t.binaryExpression(">", len, start), - t.binaryExpression("-", len, start), - t.numericLiteral(0), - ); - } + let target = path + .getEarliestCommonAncestorFrom(state.references) + .getStatementParent(); - const loop = buildRest({ - ARGUMENTS: argsId, - ARRAY_KEY: arrKey, - ARRAY_LEN: arrLen, - START: start, - ARRAY: rest, - KEY: key, - LEN: len, + // don't perform the allocation inside a loop + target.findParent(path => { + if (path.isLoop()) { + target = path; + } else { + // Stop crawling up if this is a function. + return path.isFunction(); + } }); - if (state.deopted) { - loop._blockHoist = node.params.length + 1; - node.body.body.unshift(loop); - } else { - // perform allocation at the lowest common ancestor of all references - loop._blockHoist = 1; - - let target = path - .getEarliestCommonAncestorFrom(state.references) - .getStatementParent(); - - // don't perform the allocation inside a loop - target.findParent(path => { - if (path.isLoop()) { - target = path; - } else { - // Stop crawling up if this is a function. - return path.isFunction(); - } - }); - - target.insertBefore(loop); - } - }, -}; + target.insertBefore(loop); + } +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/actual.js new file mode 100644 index 0000000000..ea2ac402c2 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/actual.js @@ -0,0 +1,8 @@ +function fn( + a1, + a2 = 4, + {a3, a4}, + a5, + {a6, a7} = {}) { + +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/expected.js new file mode 100644 index 0000000000..61953a101d --- /dev/null +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/default-rest-mix/expected.js @@ -0,0 +1,11 @@ +function fn(a1) { + var a2 = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 4; + var _arguments$ = arguments[2], + a3 = _arguments$.a3, + a4 = _arguments$.a4; + var a5 = arguments[3]; + + var _ref = arguments.length > 4 && arguments[4] !== undefined ? arguments[4] : {}, + a6 = _ref.a6, + a7 = _ref.a7; +} \ No newline at end of file diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/destructuring-rest/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/destructuring-rest/expected.js index 13f1b457d7..06a343bc15 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/destructuring-rest/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/destructuring-rest/expected.js @@ -1,9 +1,9 @@ // #3861 function t() { var x = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : "default"; - var _ref = arguments[1]; - var a = _ref.a, - b = _ref.b; + var _arguments$ = arguments[1], + a = _arguments$.a, + b = _arguments$.b; for (var _len = arguments.length, args = Array(_len > 2 ? _len - 2 : 0), _key = 2; _key < _len; _key++) { args[_key - 2] = arguments[_key]; From 8e19a5b057f5ea67c73641da3fbeb5293abd4c3f Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Tue, 2 May 2017 21:24:35 -0700 Subject: [PATCH 2/5] Update param scope values when expanding parameters. --- .../src/index.js | 9 +++++++-- .../src/params.js | 4 +++- .../src/rest.js | 6 ++++-- .../src/path/inference/inferers.js | 20 +++++++++++++++---- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/index.js b/packages/babel-plugin-transform-es2015-parameters/src/index.js index f3ffba2755..267c5f13b6 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/index.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/index.js @@ -15,8 +15,13 @@ export default function() { path.arrowFunctionToExpression(); } - convertFunctionRest(path); - convertFunctionParams(path); + const convertedRest = convertFunctionRest(path); + const convertedParams = convertFunctionParams(path); + + if (convertedRest || convertedParams) { + // Manually reprocess this scope to ensure that the moved params are updated. + path.scope.crawl(); + } }, }, }; diff --git a/packages/babel-plugin-transform-es2015-parameters/src/params.js b/packages/babel-plugin-transform-es2015-parameters/src/params.js index f6cf4b1e31..99e80cc7f6 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/params.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/params.js @@ -96,7 +96,7 @@ export default function convertFunctionParams(path) { } } - if (body.length === 0) return; + if (body.length === 0) return false; // we need to cut off all trailing parameters if (firstOptionalIndex !== null) { @@ -112,4 +112,6 @@ export default function convertFunctionParams(path) { } else { path.get("body").unshiftContainer("body", body); } + + return true; } diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index c9f7f2906c..5463633259 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -222,7 +222,7 @@ function optimiseLengthGetter(path, argsId, offset) { export default function convertFunctionRest(path) { const { node, scope } = path; - if (!hasRest(node)) return; + if (!hasRest(node)) return false; const rest = node.params.pop().argument; @@ -273,7 +273,7 @@ export default function convertFunctionRest(path) { path.replaceWith(argsId); } } - return; + return true; } state.references = state.references.concat( @@ -338,4 +338,6 @@ export default function convertFunctionRest(path) { target.insertBefore(loop); } + + return true; } diff --git a/packages/babel-traverse/src/path/inference/inferers.js b/packages/babel-traverse/src/path/inference/inferers.js index 740f3f07c8..aa2ecec29f 100644 --- a/packages/babel-traverse/src/path/inference/inferers.js +++ b/packages/babel-traverse/src/path/inference/inferers.js @@ -5,11 +5,23 @@ export { default as Identifier } from "./inferer-reference"; export function VariableDeclarator() { const id = this.get("id"); - if (id.isIdentifier()) { - return this.get("init").getTypeAnnotation(); - } else { - return; + if (!id.isIdentifier()) return; + const init = this.get("init"); + + let type = init.getTypeAnnotation(); + + if (type && type.type === "AnyTypeAnnotation") { + // Detect "var foo = Array()" calls so we can optimize for arrays vs iterables. + if ( + init.isCallExpression() && + init.get("callee").isIdentifier({ name: "Array" }) && + !init.scope.hasBinding("Array", true /* noGlobals */) + ) { + type = ArrayExpression(); + } } + + return type; } export function TypeCastExpression(node) { From 18084db7cfa093906ba8e1775645f66b58e9fa85 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Tue, 2 May 2017 21:43:24 -0700 Subject: [PATCH 3/5] Fix an ordering bug in object-rest-spread. --- packages/babel-plugin-transform-object-rest-spread/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-plugin-transform-object-rest-spread/src/index.js b/packages/babel-plugin-transform-object-rest-spread/src/index.js index 71f2029593..2583ff1d13 100644 --- a/packages/babel-plugin-transform-object-rest-spread/src/index.js +++ b/packages/babel-plugin-transform-object-rest-spread/src/index.js @@ -129,7 +129,7 @@ export default function({ types: t }) { // function a({ b, ...c }) {} Function(path) { const params = path.get("params"); - for (let i = 0; i < params.length; i++) { + for (let i = params.length - 1; i >= 0; i--) { replaceRestElement(params[i].parentPath, params[i], i, params.length); } }, From d86ae2fb84f442c915174a23bef3097a26ed2819 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Tue, 2 May 2017 13:14:56 -0700 Subject: [PATCH 4/5] Remove _blockHoist usage from param processing. --- .../babel-plugin-transform-es2015-parameters/src/params.js | 3 --- .../babel-plugin-transform-es2015-parameters/src/rest.js | 4 ---- .../babel-plugin-transform-object-rest-spread/src/index.js | 1 - .../dont-hoist-before-default-params/expected.js | 5 ++--- 4 files changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/params.js b/packages/babel-plugin-transform-es2015-parameters/src/params.js index 99e80cc7f6..de87c35d8f 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/params.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/params.js @@ -73,11 +73,9 @@ export default function convertFunctionParams(path) { DEFAULT_VALUE: right.node, ARGUMENT_KEY: t.numericLiteral(i), }); - defNode._blockHoist = node.params.length - i; body.push(defNode); } else if (firstOptionalIndex !== null) { const defNode = buildArgumentsAccess(param.node, t.numericLiteral(i)); - defNode._blockHoist = node.params.length - i; body.push(defNode); } else if (param.isObjectPattern() || param.isArrayPattern()) { const uid = path.scope.generateUidIdentifier("ref"); @@ -85,7 +83,6 @@ export default function convertFunctionParams(path) { const defNode = t.variableDeclaration("let", [ t.variableDeclarator(param.node, uid), ]); - defNode._blockHoist = node.params.length - i; body.push(defNode); param.replaceWith(uid); diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index 5463633259..4617f8f5d2 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -316,12 +316,8 @@ export default function convertFunctionRest(path) { }); if (state.deopted) { - loop._blockHoist = node.params.length + 1; node.body.body.unshift(loop); } else { - // perform allocation at the lowest common ancestor of all references - loop._blockHoist = 1; - let target = path .getEarliestCommonAncestorFrom(state.references) .getStatementParent(); diff --git a/packages/babel-plugin-transform-object-rest-spread/src/index.js b/packages/babel-plugin-transform-object-rest-spread/src/index.js index 2583ff1d13..c96358fed1 100644 --- a/packages/babel-plugin-transform-object-rest-spread/src/index.js +++ b/packages/babel-plugin-transform-object-rest-spread/src/index.js @@ -113,7 +113,6 @@ export default function({ types: t }) { const declar = t.variableDeclaration("let", [ t.variableDeclarator(paramPath.node, uid), ]); - declar._blockHoist = i ? numParams - i : 1; parentPath.ensureBlock(); parentPath.get("body").unshiftContainer("body", declar); diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-default-params/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-default-params/expected.js index a11f2f892a..6780bc3d89 100644 --- a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-default-params/expected.js +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-default-params/expected.js @@ -1,7 +1,6 @@ function render(Component) { - var text = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : ''; - - var _ref = ; + var text = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : '', + _ref = ; return function () { return _ref; From 9dd65c809f884d320355ebbe6163a1b43454f2a3 Mon Sep 17 00:00:00 2001 From: Brian Ng Date: Thu, 3 Aug 2017 22:27:30 -0500 Subject: [PATCH 5/5] fixes --- .../src/index.js | 2 +- .../src/params.js | 50 ++++++++++++++++++- .../parameters/rest-nested-5656/expected.js | 2 +- .../object-rest/parameters/expected.js | 12 ++--- 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/index.js b/packages/babel-plugin-transform-es2015-parameters/src/index.js index 267c5f13b6..af12fa0399 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/index.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/index.js @@ -16,7 +16,7 @@ export default function() { } const convertedRest = convertFunctionRest(path); - const convertedParams = convertFunctionParams(path); + const convertedParams = convertFunctionParams(path, this.opts.loose); if (convertedRest || convertedParams) { // Manually reprocess this scope to ensure that the moved params are updated. diff --git a/packages/babel-plugin-transform-es2015-parameters/src/params.js b/packages/babel-plugin-transform-es2015-parameters/src/params.js index de87c35d8f..73215cbd09 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/params.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/params.js @@ -10,6 +10,16 @@ const buildDefaultParam = template(` DEFAULT_VALUE; `); +const buildLooseDefaultParam = template(` + if (ASSIGNMENT_IDENTIFIER === UNDEFINED) { + ASSIGNMENT_IDENTIFIER = DEFAULT_VALUE; + } +`); + +const buildLooseDestructuredDefaultParam = template(` + let ASSIGNMENT_IDENTIFIER = PARAMETER_NAME === UNDEFINED ? DEFAULT_VALUE : PARAMETER_NAME ; +`); + const buildArgumentsAccess = template(` let $0 = arguments[$1]; `); @@ -35,7 +45,7 @@ const iifeVisitor = { }, }; -export default function convertFunctionParams(path) { +export default function convertFunctionParams(path, loose) { const { node, scope } = path; const state = { @@ -49,6 +59,44 @@ export default function convertFunctionParams(path) { // const params = path.get("params"); + + if (loose) { + const body = []; + for (let i = 0; i < params.length; ++i) { + const param = params[i]; + if (param.isAssignmentPattern()) { + const left = param.get("left"); + const right = param.get("right"); + + const undefinedNode = scope.buildUndefinedNode(); + + if (left.isIdentifier()) { + body.push( + buildLooseDefaultParam({ + ASSIGNMENT_IDENTIFIER: left.node, + DEFAULT_VALUE: right.node, + UNDEFINED: undefinedNode, + }), + ); + param.replaceWith(left.node); + } else if (left.isObjectPattern() || left.isArrayPattern()) { + const paramName = scope.generateUidIdentifier(); + body.push( + buildLooseDestructuredDefaultParam({ + ASSIGNMENT_IDENTIFIER: left.node, + DEFAULT_VALUE: right.node, + PARAMETER_NAME: paramName, + UNDEFINED: undefinedNode, + }), + ); + param.replaceWith(paramName); + } + } + } + path.get("body").unshiftContainer("body", body); + return; + } + for (let i = 0; i < params.length; i++) { const param = params[i]; diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-nested-5656/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-nested-5656/expected.js index c0d587ed6e..1a76699de9 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-nested-5656/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-nested-5656/expected.js @@ -28,7 +28,7 @@ function d(thing) { }; { - var _args = thing; + var args = thing; foo(thing); } } diff --git a/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/parameters/expected.js b/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/parameters/expected.js index a24b010661..39cbd5c047 100644 --- a/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/parameters/expected.js +++ b/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/parameters/expected.js @@ -17,15 +17,15 @@ function a3(_ref3) { c2 = babelHelpers.objectWithoutProperties(_ref3, ["a2", "b2"]); } -function a4(_ref4, _ref5) { - let { - a5 - } = _ref5, - c5 = babelHelpers.objectWithoutProperties(_ref5, ["a5"]); +function a4(_ref5, _ref4) { let { a3 + } = _ref5, + c3 = babelHelpers.objectWithoutProperties(_ref5, ["a3"]); + let { + a5 } = _ref4, - c3 = babelHelpers.objectWithoutProperties(_ref4, ["a3"]); + c5 = babelHelpers.objectWithoutProperties(_ref4, ["a5"]); } function a5(_ref6) {