From 4ca686b7be8d214aa6879c2cf10d328633a1792a Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Mon, 14 Aug 2017 22:22:18 -0400 Subject: [PATCH] Fix relative execution location introspection (#5741) So, I was reading the new Flow type strictness and noticed https://flow.org/blog/2017/05/07/Strict-Function-Call-Arity/ Specifically, I wondered whether the `sum_all` example would copy the arguments into an array, then loop over. Sadly, it does. ```js function sum_all(...rest) { let ret = 0; for (let i = 0; i < rest.length; i++) { ret += rest[i]; } return ret; } // output function sum_all() { var ret = 0; for (var _len = arguments.length, rest = Array(_len), _key = 0; _key < _len; _key++) { rest[_key] = arguments[_key]; } for (var i = 0; i < rest.length; i++) { ret += rest[i]; } return ret; } ``` But then I noticed if I changed `let i = 0` to `let i: number = 0`, it worked directly on `arguments`. That lead me down a rabbit hole to `Path#_guessExecutionStatusRelativeTo`. When tracing through, the last comparison made no sense to me. It was trying to find the index of `"init"` in a list of `["declarations"]` and `"body"` in `["directives", "body"]`. Red flags and such. But it makes sense when you're trying to compare the visitor order of the common ancestor path. Then we're trying to find `"init"` in a list of `["init", "test", "update", "body"]`. Oh, and there's `"body"` in there too! And now we know the `ForStatement`'s `init` is executed before the `body`. --- .../rest-member-expression-optimisation/expected.js | 8 ++------ packages/babel-traverse/src/path/introspection.js | 9 +++------ 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-optimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-optimisation/expected.js index 7830720e3e..62aab77ce7 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-optimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-optimisation/expected.js @@ -20,11 +20,7 @@ function t() { function t() { - for (var _len = arguments.length, items = Array(_len), _key = 0; _key < _len; _key++) { - items[_key] = arguments[_key]; - } - - for (var i = 0; i < items.length; i++) { - return items[i]; + for (var i = 0; i < arguments.length; i++) { + return arguments.length <= i ? undefined : arguments[i]; } } diff --git a/packages/babel-traverse/src/path/introspection.js b/packages/babel-traverse/src/path/introspection.js index 1888a6e2af..a72eca0078 100644 --- a/packages/babel-traverse/src/path/introspection.js +++ b/packages/babel-traverse/src/path/introspection.js @@ -270,12 +270,9 @@ export function _guessExecutionStatusRelativeTo(target) { } // otherwise we're associated by a parent node, check which key comes before the other - const targetKeyPosition = t.VISITOR_KEYS[targetRelationship.type].indexOf( - targetRelationship.key, - ); - const selfKeyPosition = t.VISITOR_KEYS[selfRelationship.type].indexOf( - selfRelationship.key, - ); + const keys = t.VISITOR_KEYS[commonPath.type]; + const targetKeyPosition = keys.indexOf(targetRelationship.key); + const selfKeyPosition = keys.indexOf(selfRelationship.key); return targetKeyPosition > selfKeyPosition ? "before" : "after"; }