From 844dd33f3d2744f20b29e398575950704d55d41a Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Sat, 24 Nov 2018 09:32:24 -0500 Subject: [PATCH] Microbouji patch/8136 (#9073) * Fix optional chaining bug regarding spread in function calls * Revamp optional-chain to be top down Instead of going both upwards and downwards from the first real optional expression, we can just start from the top down. * Add more tests --- .../src/index.js | 225 ++++++++---------- .../general/function-call-spread/input.js | 7 + .../general/function-call-spread/options.json | 7 + .../general/function-call-spread/output.js | 6 + 4 files changed, 123 insertions(+), 122 deletions(-) create mode 100644 packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/input.js create mode 100644 packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/options.json create mode 100644 packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/output.js diff --git a/packages/babel-plugin-proposal-optional-chaining/src/index.js b/packages/babel-plugin-proposal-optional-chaining/src/index.js index 586e334f3c..c3e3240415 100644 --- a/packages/babel-plugin-proposal-optional-chaining/src/index.js +++ b/packages/babel-plugin-proposal-optional-chaining/src/index.js @@ -7,136 +7,117 @@ export default declare((api, options) => { const { loose = false } = options; - function optional(path, replacementPath) { - const { scope } = path; - const optionals = []; - - let objectPath = path; - while ( - objectPath.isOptionalMemberExpression() || - objectPath.isOptionalCallExpression() - ) { - const { node } = objectPath; - if (node.optional) { - optionals.push(node); - } - - if (objectPath.isOptionalMemberExpression()) { - objectPath.node.type = "MemberExpression"; - objectPath = objectPath.get("object"); - } else { - objectPath.node.type = "CallExpression"; - objectPath = objectPath.get("callee"); - } - } - - for (let i = optionals.length - 1; i >= 0; i--) { - const node = optionals[i]; - node.optional = false; - - const isCall = t.isCallExpression(node); - const replaceKey = isCall ? "callee" : "object"; - const chain = node[replaceKey]; - - let ref; - let check; - if (loose && isCall) { - // If we are using a loose transform (avoiding a Function#call) and we are at the call, - // we can avoid a needless memoize. - check = ref = chain; - } else { - ref = scope.maybeGenerateMemoised(chain); - if (ref) { - check = t.assignmentExpression( - "=", - t.cloneNode(ref), - // Here `chain` MUST NOT be cloned because it could be updated - // when generating the memoised context of a call espression - chain, - ); - node[replaceKey] = ref; - } else { - check = ref = chain; - } - } - - // Ensure call expressions have the proper `this` - // `foo.bar()` has context `foo`. - if (isCall && t.isMemberExpression(chain)) { - if (loose) { - // To avoid a Function#call, we can instead re-grab the property from the context object. - // `a.?b.?()` translates roughly to `_a.b != null && _a.b()` - node.callee = chain; - } else { - // Otherwise, we need to memoize the context object, and change the call into a Function#call. - // `a.?b.?()` translates roughly to `(_b = _a.b) != null && _b.call(_a)` - const { object } = chain; - let context = scope.maybeGenerateMemoised(object); - if (context) { - chain.object = t.assignmentExpression("=", context, object); - } else { - context = object; - } - - node.arguments.unshift(t.cloneNode(context)); - node.callee = t.memberExpression(node.callee, t.identifier("call")); - } - } - - replacementPath.replaceWith( - t.conditionalExpression( - loose - ? t.binaryExpression("==", t.cloneNode(check), t.nullLiteral()) - : t.logicalExpression( - "||", - t.binaryExpression("===", t.cloneNode(check), t.nullLiteral()), - t.binaryExpression( - "===", - t.cloneNode(ref), - scope.buildUndefinedNode(), - ), - ), - scope.buildUndefinedNode(), - replacementPath.node, - ), - ); - - replacementPath = replacementPath.get("alternate"); - } - } - - function findReplacementPath(path) { - return path.find(path => { - const { parentPath } = path; - - if (path.key == "object" && parentPath.isOptionalMemberExpression()) { - return false; - } - if (path.key == "callee" && parentPath.isOptionalCallExpression()) { - return false; - } - if ( - path.key == "argument" && - parentPath.isUnaryExpression({ operator: "delete" }) - ) { - return false; - } - - return true; - }); - } - return { name: "proposal-optional-chaining", inherits: syntaxOptionalChaining, visitor: { "OptionalCallExpression|OptionalMemberExpression"(path) { - if (!path.node.optional) { - return; + const { parentPath, scope } = path; + const optionals = []; + + let optionalPath = path; + while ( + optionalPath.isOptionalMemberExpression() || + optionalPath.isOptionalCallExpression() + ) { + const { node } = optionalPath; + if (node.optional) { + optionals.push(node); + } + + if (optionalPath.isOptionalMemberExpression()) { + optionalPath.node.type = "MemberExpression"; + optionalPath = optionalPath.get("object"); + } else if (optionalPath.isOptionalCallExpression()) { + optionalPath.node.type = "CallExpression"; + optionalPath = optionalPath.get("callee"); + } } - optional(path, findReplacementPath(path)); + let replacementPath = path; + if (parentPath.isUnaryExpression({ operator: "delete" })) { + replacementPath = parentPath; + } + for (let i = optionals.length - 1; i >= 0; i--) { + const node = optionals[i]; + + const isCall = t.isCallExpression(node); + const replaceKey = isCall ? "callee" : "object"; + const chain = node[replaceKey]; + + let ref; + let check; + if (loose && isCall) { + // If we are using a loose transform (avoiding a Function#call) and we are at the call, + // we can avoid a needless memoize. + check = ref = chain; + } else { + ref = scope.maybeGenerateMemoised(chain); + if (ref) { + check = t.assignmentExpression( + "=", + t.cloneNode(ref), + // Here `chain` MUST NOT be cloned because it could be updated + // when generating the memoised context of a call espression + chain, + ); + node[replaceKey] = ref; + } else { + check = ref = chain; + } + } + + // Ensure call expressions have the proper `this` + // `foo.bar()` has context `foo`. + if (isCall && t.isMemberExpression(chain)) { + if (loose) { + // To avoid a Function#call, we can instead re-grab the property from the context object. + // `a.?b.?()` translates roughly to `_a.b != null && _a.b()` + node.callee = chain; + } else { + // Otherwise, we need to memoize the context object, and change the call into a Function#call. + // `a.?b.?()` translates roughly to `(_b = _a.b) != null && _b.call(_a)` + const { object } = chain; + let context = scope.maybeGenerateMemoised(object); + if (context) { + chain.object = t.assignmentExpression("=", context, object); + } else { + context = object; + } + + node.arguments.unshift(t.cloneNode(context)); + node.callee = t.memberExpression( + node.callee, + t.identifier("call"), + ); + } + } + + replacementPath.replaceWith( + t.conditionalExpression( + loose + ? t.binaryExpression("==", t.cloneNode(check), t.nullLiteral()) + : t.logicalExpression( + "||", + t.binaryExpression( + "===", + t.cloneNode(check), + t.nullLiteral(), + ), + t.binaryExpression( + "===", + t.cloneNode(ref), + scope.buildUndefinedNode(), + ), + ), + scope.buildUndefinedNode(), + replacementPath.node, + ), + ); + + replacementPath = replacementPath.get("alternate"); + } }, }, }; diff --git a/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/input.js b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/input.js new file mode 100644 index 0000000000..6094ab401b --- /dev/null +++ b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/input.js @@ -0,0 +1,7 @@ +a?.(...args); + +a?.b(...args); + +a?.b(...args).c; + +a?.b(...args).c(...args); diff --git a/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/options.json b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/options.json new file mode 100644 index 0000000000..e2efd1fae6 --- /dev/null +++ b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/options.json @@ -0,0 +1,7 @@ +{ + "plugins": [ + "external-helpers", + "proposal-optional-chaining", + "transform-spread" + ] +} diff --git a/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/output.js b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/output.js new file mode 100644 index 0000000000..9c180568c5 --- /dev/null +++ b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/output.js @@ -0,0 +1,6 @@ +var _a, _a2, _a3, _a4, _a4$b; + +(_a = a) === null || _a === void 0 ? void 0 : _a.apply(void 0, babelHelpers.toConsumableArray(args)); +(_a2 = a) === null || _a2 === void 0 ? void 0 : _a2.b.apply(_a2, babelHelpers.toConsumableArray(args)); +(_a3 = a) === null || _a3 === void 0 ? void 0 : _a3.b.apply(_a3, babelHelpers.toConsumableArray(args)).c; +(_a4 = a) === null || _a4 === void 0 ? void 0 : (_a4$b = _a4.b.apply(_a4, babelHelpers.toConsumableArray(args))).c.apply(_a4$b, babelHelpers.toConsumableArray(args));