Skip to content

Commit

Permalink
Fix rest parameters indexing with TypeScript 'this parameter' (#9714)
Browse files Browse the repository at this point in the history
* 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

* 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
  • Loading branch information
BenoitZugmeyer authored and nicolo-ribaudo committed Jan 17, 2020
1 parent 34a9653 commit 85ddc29
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 3 deletions.
18 changes: 15 additions & 3 deletions packages/babel-plugin-transform-parameters/src/rest.js
Expand Up @@ -155,6 +155,16 @@ const memberExpressionOptimisationVisitor = {
}
},
};

function getParamsCount(node) {
let count = node.params.length;
// skip the first parameter if it is a TypeScript 'this parameter'
if (count > 0 && t.isIdentifier(node.params[0], { name: "this" })) {
count -= 1;
}
return count;
}

function hasRest(node) {
const length = node.params.length;
return length > 0 && t.isRestElement(node.params[length - 1]);
Expand Down Expand Up @@ -244,10 +254,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),
Expand Down Expand Up @@ -297,12 +309,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
Expand Down
Expand Up @@ -2,6 +2,7 @@
"plugins": [
"proposal-class-properties",
"external-helpers",
"syntax-typescript",
"syntax-flow",
"transform-parameters",
"transform-block-scoping",
Expand Down
@@ -0,0 +1,7 @@
function u(this: Foo, ...items) {
items[0];
}

function v(this: Foo, event: string, ...args: any[]) {
args;
}
@@ -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;
}

0 comments on commit 85ddc29

Please sign in to comment.