From 63dd253df7f344fed5b643c686b289b6b75842de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Mon, 23 Dec 2019 12:33:21 -0500 Subject: [PATCH] address review comments --- packages/babel-traverse/src/path/lib/virtual-types.js | 3 ++- packages/babel-traverse/src/scope/index.js | 4 ++-- packages/babel-traverse/src/scope/lib/renamer.js | 3 +-- packages/babel-traverse/test/scope.js | 6 +++--- packages/babel-types/src/validators/isScope.js | 2 ++ 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/babel-traverse/src/path/lib/virtual-types.js b/packages/babel-traverse/src/path/lib/virtual-types.js index 97cec0e3af99..d7072d6f6e4a 100644 --- a/packages/babel-traverse/src/path/lib/virtual-types.js +++ b/packages/babel-traverse/src/path/lib/virtual-types.js @@ -64,7 +64,8 @@ export const Expression = { }; export const Scope = { - types: ["Scopable"], + // When pattern is inside the function params, it is a scope + types: ["Scopable", "Pattern"], checkPath(path) { return t.isScope(path.node, path.parent); }, diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index d1ebaf15a099..9f9d78667336 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -902,14 +902,14 @@ export default class Scope { do { const binding = scope.getOwnBinding(name); if (binding) { - // When a pattern is a Scope, it is a part of parameter expressions. + // Check if a pattern is a part of parameter expressions. // 9.2.10.28: The closure created by this expression should not have visibility of // declarations in the function body. If the binding is not a `param`-kind, // then it must be defined inside the function body, thus it should be skipped if ( previousPath && previousPath.isPattern() && - previousPath.isScope() && + previousPath.parentPath.isFunction() && binding.kind !== "param" ) { // do nothing diff --git a/packages/babel-traverse/src/scope/lib/renamer.js b/packages/babel-traverse/src/scope/lib/renamer.js index 98c16a2e1863..cdfd90819681 100644 --- a/packages/babel-traverse/src/scope/lib/renamer.js +++ b/packages/babel-traverse/src/scope/lib/renamer.js @@ -9,9 +9,8 @@ const renameVisitor = { } }, - "Pattern|Scope"(path, state) { + Scope(path, state) { if ( - path.isScope() && !path.scope.bindingIdentifierEquals( state.oldName, state.binding.identifier, diff --git a/packages/babel-traverse/test/scope.js b/packages/babel-traverse/test/scope.js index a40402a2d924..b796dbf682f5 100644 --- a/packages/babel-traverse/test/scope.js +++ b/packages/babel-traverse/test/scope.js @@ -73,8 +73,8 @@ describe("scope", () => { ).toBe("Identifier"); }); - describe("function paramater expression", function() { - it("should not has visibility of declarations inside function body", () => { + describe("function parameter expression", function() { + it("should not have visibility of declarations inside function body", () => { expect( getPath( `var a = "outside"; (function foo(b = a) { let a = "inside" })`, @@ -83,7 +83,7 @@ describe("scope", () => { .scope.getBinding("a").path.node.init.value, ).toBe("outside"); }); - it("should has visibility on paramater bindings", () => { + it("should have visibility on parameter bindings", () => { expect( getPath(`var a = "outside"; (function foo(b = a, a = "inside") {})`) .get("body.1.expression.params.0") diff --git a/packages/babel-types/src/validators/isScope.js b/packages/babel-types/src/validators/isScope.js index e7ddcb2ce3ef..dfd2f5edd96f 100644 --- a/packages/babel-types/src/validators/isScope.js +++ b/packages/babel-types/src/validators/isScope.js @@ -19,6 +19,8 @@ export default function isScope(node: Object, parent: Object): boolean { return false; } + // If a Pattern is an immediate descendent of a Function, it must be in the params. + // Hence we skipped the parentKey === "params" check if (isPattern(node) && isFunction(parent)) { return true; }