From aa64170fd76c6f2a642195b7fe5c27f48df55ccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 20 Mar 2019 14:02:16 +0100 Subject: [PATCH 1/2] Fix rest parameters indexing with TypeScript 'this parameter' The TypeScript [this parameter][0] is a fake parameter and should not be taken into account when counting the function parameters. This patch skips it by using the condition taken from the `transform-typescript` plugin. Note: since the `transform-typescript` plugin is removing this kind of parameter, including it before the `transform-parameters` plugin solves the issue. This patch fixes the issue in other cases. [0]: https://www.typescriptlang.org/docs/handbook/functions.html#this-parameters --- .../src/rest.js | 19 ++++++++++++++++--- .../test/fixtures/parameters/options.json | 1 + .../rest-ts-this-parameter/input.js | 7 +++++++ .../rest-ts-this-parameter/output.js | 11 +++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/input.js create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/output.js diff --git a/packages/babel-plugin-transform-parameters/src/rest.js b/packages/babel-plugin-transform-parameters/src/rest.js index da85c7e5ad95..a8615d062394 100644 --- a/packages/babel-plugin-transform-parameters/src/rest.js +++ b/packages/babel-plugin-transform-parameters/src/rest.js @@ -153,6 +153,17 @@ const memberExpressionOptimisationVisitor = { } }, }; + +function getParamsCount(node) { + let count = node.params.length; + // skip the first parameter if it is a TypeScript 'this parameter' + const p0 = node.params[0]; + if (p0 && t.isIdentifier(p0) && p0.name === "this") { + count -= 1; + } + return count; +} + function hasRest(node) { const length = node.params.length; return length > 0 && t.isRestElement(node.params[length - 1]); @@ -242,10 +253,12 @@ export default function convertFunctionRest(path) { node.body.body.unshift(declar); } + const paramsCount = getParamsCount(node); + // check and optimise for extremely common cases const state = { references: [], - offset: node.params.length, + offset: paramsCount, argumentsNode: argsId, outerBinding: scope.getBindingIdentifier(rest.name), @@ -295,12 +308,12 @@ export default function convertFunctionRest(path) { state.candidates.map(({ path }) => path), ); - const start = t.numericLiteral(node.params.length); + const start = t.numericLiteral(paramsCount); const key = scope.generateUidIdentifier("key"); const len = scope.generateUidIdentifier("len"); let arrKey, arrLen; - if (node.params.length) { + if (paramsCount) { // 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 diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/options.json b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/options.json index 4ee046b12d4c..4698618caab7 100644 --- a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/options.json +++ b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/options.json @@ -2,6 +2,7 @@ "plugins": [ "proposal-class-properties", "external-helpers", + "syntax-typescript", "syntax-flow", "transform-parameters", "transform-block-scoping", diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/input.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/input.js new file mode 100644 index 000000000000..1d2875453ecd --- /dev/null +++ b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/input.js @@ -0,0 +1,7 @@ +function u(this: Foo, ...items) { + items[0]; +} + +function v(this: Foo, event: string, ...args: any[]) { + args; +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/output.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/output.js new file mode 100644 index 000000000000..55822308fd13 --- /dev/null +++ b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/rest-ts-this-parameter/output.js @@ -0,0 +1,11 @@ +function u(this: Foo) { + arguments.length <= 0 ? undefined : arguments[0]; +} + +function v(this: Foo, event: string) { + for (var _len = arguments.length, args = new Array(_len > 1 ? _len - 1 : 0), _key = 1; _key < _len; _key++) { + args[_key - 1] = arguments[_key]; + } + + args; +} From a25b6218c47cb68f3a2b0126b6854d5785d9ff7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 13 Jan 2020 12:41:39 +0100 Subject: [PATCH 2/2] nit: improve the 'this parameter' detection condition * improve performance by checking the parameter list length before accessing it * simplify the test a bit by using the `isIdentifier` second parameter --- packages/babel-plugin-transform-parameters/src/rest.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/babel-plugin-transform-parameters/src/rest.js b/packages/babel-plugin-transform-parameters/src/rest.js index 028e72673639..cfba76ce4ae4 100644 --- a/packages/babel-plugin-transform-parameters/src/rest.js +++ b/packages/babel-plugin-transform-parameters/src/rest.js @@ -159,8 +159,7 @@ const memberExpressionOptimisationVisitor = { function getParamsCount(node) { let count = node.params.length; // skip the first parameter if it is a TypeScript 'this parameter' - const p0 = node.params[0]; - if (p0 && t.isIdentifier(p0) && p0.name === "this") { + if (count > 0 && t.isIdentifier(node.params[0], { name: "this" })) { count -= 1; } return count;