From da0735d2151c097a61ce6d6bc0a44fe577d67c75 Mon Sep 17 00:00:00 2001 From: Elian Ibaj Date: Fri, 22 Jun 2018 12:00:16 +0200 Subject: [PATCH 1/3] Fix optional chaining bug regarding spread in function calls --- packages/babel-plugin-proposal-optional-chaining/src/index.js | 4 ++++ .../test/fixtures/general/function-call-spread/input.js | 3 +++ .../test/fixtures/general/function-call-spread/options.json | 3 +++ .../test/fixtures/general/function-call-spread/output.js | 4 ++++ 4 files changed, 14 insertions(+) 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 586e334f3ca4..84948a21cf8b 100644 --- a/packages/babel-plugin-proposal-optional-chaining/src/index.js +++ b/packages/babel-plugin-proposal-optional-chaining/src/index.js @@ -83,6 +83,10 @@ export default declare((api, options) => { } } + if (replacementPath.isOptionalCallExpression()) { + replacementPath.node.type = "CallExpression"; + } + replacementPath.replaceWith( t.conditionalExpression( loose 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 000000000000..b8eff3f37679 --- /dev/null +++ b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/input.js @@ -0,0 +1,3 @@ +a?.(...args); + +a?.b(...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 000000000000..012460630a9b --- /dev/null +++ b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/options.json @@ -0,0 +1,3 @@ +{ + "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 000000000000..91c60ba9d349 --- /dev/null +++ b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/output.js @@ -0,0 +1,4 @@ +var _a, _a2; + +(_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)); From 9e88d3628e1b1e8d57565c0742f41ee2e7c1f69d Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Fri, 23 Nov 2018 19:16:10 -0500 Subject: [PATCH 2/3] 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. --- .../src/index.js | 225 ++++++++---------- .../general/function-call-spread/options.json | 6 +- 2 files changed, 106 insertions(+), 125 deletions(-) diff --git a/packages/babel-plugin-proposal-optional-chaining/src/index.js b/packages/babel-plugin-proposal-optional-chaining/src/index.js index 84948a21cf8b..c3e324041520 100644 --- a/packages/babel-plugin-proposal-optional-chaining/src/index.js +++ b/packages/babel-plugin-proposal-optional-chaining/src/index.js @@ -7,140 +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; + return { + name: "proposal-optional-chaining", + inherits: syntaxOptionalChaining, - const isCall = t.isCallExpression(node); - const replaceKey = isCall ? "callee" : "object"; - const chain = node[replaceKey]; + visitor: { + "OptionalCallExpression|OptionalMemberExpression"(path) { + const { parentPath, scope } = path; + const optionals = []; + + let optionalPath = path; + while ( + optionalPath.isOptionalMemberExpression() || + optionalPath.isOptionalCallExpression() + ) { + const { node } = optionalPath; + if (node.optional) { + optionals.push(node); + } - 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; + if (optionalPath.isOptionalMemberExpression()) { + optionalPath.node.type = "MemberExpression"; + optionalPath = optionalPath.get("object"); + } else if (optionalPath.isOptionalCallExpression()) { + optionalPath.node.type = "CallExpression"; + optionalPath = optionalPath.get("callee"); + } } - } - // 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); + 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 { - context = object; + 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; + } } - node.arguments.unshift(t.cloneNode(context)); - node.callee = t.memberExpression(node.callee, t.identifier("call")); - } - } - - if (replacementPath.isOptionalCallExpression()) { - replacementPath.node.type = "CallExpression"; - } - - 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; - }); - } + // 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"), + ); + } + } - return { - name: "proposal-optional-chaining", - inherits: syntaxOptionalChaining, + 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, + ), + ); - visitor: { - "OptionalCallExpression|OptionalMemberExpression"(path) { - if (!path.node.optional) { - return; + replacementPath = replacementPath.get("alternate"); } - - optional(path, findReplacementPath(path)); }, }, }; 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 index 012460630a9b..e2efd1fae654 100644 --- 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 @@ -1,3 +1,7 @@ { - "plugins": ["external-helpers", "proposal-optional-chaining", "transform-spread"] + "plugins": [ + "external-helpers", + "proposal-optional-chaining", + "transform-spread" + ] } From 38a4466f7053063f19ad4ec070cc8fa631e20684 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Fri, 23 Nov 2018 19:23:44 -0500 Subject: [PATCH 3/3] Add more tests --- .../test/fixtures/general/function-call-spread/input.js | 4 ++++ .../test/fixtures/general/function-call-spread/output.js | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) 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 index b8eff3f37679..6094ab401b98 100644 --- 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 @@ -1,3 +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/output.js b/packages/babel-plugin-proposal-optional-chaining/test/fixtures/general/function-call-spread/output.js index 91c60ba9d349..9c180568c58b 100644 --- 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 @@ -1,4 +1,6 @@ -var _a, _a2; +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));