Skip to content

Commit

Permalink
Fix handling of this&co in computed keys in arrows transform (#14005)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolo-ribaudo committed Nov 30, 2021
1 parent 8936501 commit 36a5ac4
Show file tree
Hide file tree
Showing 20 changed files with 286 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
],
"dependencies": {
"@babel/helper-annotate-as-pure": "workspace:^",
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-function-name": "workspace:^",
"@babel/helper-member-expression-to-functions": "workspace:^",
"@babel/helper-optimise-call-expression": "workspace:^",
Expand Down
11 changes: 7 additions & 4 deletions packages/babel-helper-create-class-features-plugin/src/fields.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { template, traverse, types as t } from "@babel/core";
import type { File } from "@babel/core";
import type { NodePath, Visitor, Scope } from "@babel/traverse";
import ReplaceSupers, {
environmentVisitor,
} from "@babel/helper-replace-supers";
import ReplaceSupers from "@babel/helper-replace-supers";
import environmentVisitor from "@babel/helper-environment-visitor";
import memberExpressionToFunctions from "@babel/helper-member-expression-to-functions";
import type {
Handler,
Expand Down Expand Up @@ -851,7 +850,11 @@ function buildPrivateMethodDeclaration(
);
}

const thisContextVisitor = traverse.visitors.merge([
const thisContextVisitor = traverse.visitors.merge<{
classRef: t.Identifier;
needsClassRef: boolean;
innerBinding: t.Identifier;
}>([
{
ThisExpression(path, state) {
state.needsClassRef = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { template, traverse, types as t } from "@babel/core";
import type { File } from "@babel/core";
import type { NodePath, Scope, Visitor, Binding } from "@babel/traverse";
import { environmentVisitor } from "@babel/helper-replace-supers";
import environmentVisitor from "@babel/helper-environment-visitor";

const findBareSupers = traverse.visitors.merge([
const findBareSupers = traverse.visitors.merge<NodePath<t.CallExpression>[]>([
{
Super(path: NodePath<t.Super>) {
const { node, parentPath } = path;
Expand Down
3 changes: 3 additions & 0 deletions packages/babel-helper-environment-visitor/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
src
test
*.log
19 changes: 19 additions & 0 deletions packages/babel-helper-environment-visitor/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# @babel/helper-environment-visitor

> Helper visitor to only visit nodes in the current 'this' context
See our website [@babel/helper-environment-visitor](https://babeljs.io/docs/en/babel-helper-environment-visitor) for more information.

## Install

Using npm:

```sh
npm install --save-dev @babel/helper-environment-visitor
```

or using yarn:

```sh
yarn add @babel/helper-environment-visitor --dev
```
29 changes: 29 additions & 0 deletions packages/babel-helper-environment-visitor/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "@babel/helper-environment-visitor",
"version": "7.16.0",
"description": "Helper visitor to only visit nodes in the current 'this' context",
"repository": {
"type": "git",
"url": "https://github.com/babel/babel.git",
"directory": "packages/babel-helper-environment-visitor"
},
"homepage": "https://babel.dev/docs/en/next/babel-helper-environment-visitor",
"license": "MIT",
"publishConfig": {
"access": "public"
},
"main": "./lib/index.js",
"exports": {
".": "./lib/index.js"
},
"dependencies": {
"@babel/types": "workspace:^"
},
"devDependencies": {
"@babel/traverse": "workspace:^"
},
"engines": {
"node": ">=6.9.0"
},
"author": "The Babel Team (https://babel.dev/team)"
}
38 changes: 38 additions & 0 deletions packages/babel-helper-environment-visitor/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import type { NodePath } from "@babel/traverse";
import { VISITOR_KEYS, staticBlock } from "@babel/types";
import type * as t from "@babel/types";

// TODO (Babel 8): Don't export this function.
export function skipAllButComputedKey(
path: NodePath<t.Method | t.ClassProperty>,
) {
// If the path isn't computed, just skip everything.
if (!path.node.computed) {
path.skip();
return;
}

// So it's got a computed key. Make sure to skip every other key the
// traversal would visit.
const keys = VISITOR_KEYS[path.type];
for (const key of keys) {
if (key !== "key") path.skipKey(key);
}
}

// Methods are handled by the Method visitor; arrows are not skipped because they inherit the context.
const skipKey = process.env.BABEL_8_BREAKING
? "StaticBlock|ClassPrivateProperty|TypeAnnotation|FunctionDeclaration|FunctionExpression"
: (staticBlock ? "StaticBlock|" : "") +
"ClassPrivateProperty|TypeAnnotation|FunctionDeclaration|FunctionExpression";

// environmentVisitor should be used when traversing the whole class and not for specific class elements/methods.
// For perf reasons, the environmentVisitor might be traversed with `{ noScope: true }`, which means `path.scope` is undefined.
// Avoid using `path.scope` here
export default {
[skipKey]: path => path.skip(),

"Method|ClassProperty"(path: NodePath<t.Method | t.ClassProperty>) {
skipAllButComputedKey(path);
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ export interface HandlerState<State = {}> extends Handler<State> {
handle(
this: HandlerState<State> & State,
member: Member,
noDocumentAll: boolean,
noDocumentAll?: boolean,
): void;
memoiser: AssignmentMemoiser;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-helper-module-transforms/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
},
"main": "./lib/index.js",
"dependencies": {
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-module-imports": "workspace:^",
"@babel/helper-replace-supers": "workspace:^",
"@babel/helper-simple-access": "workspace:^",
"@babel/helper-split-export-declaration": "workspace:^",
"@babel/helper-validator-identifier": "workspace:^",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { environmentVisitor } from "@babel/helper-replace-supers";
import environmentVisitor from "@babel/helper-environment-visitor";
import traverse from "@babel/traverse";
import { numericLiteral, unaryExpression } from "@babel/types";
import type * as t from "@babel/types";
Expand Down
1 change: 1 addition & 0 deletions packages/babel-helper-replace-supers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
},
"main": "./lib/index.js",
"dependencies": {
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-member-expression-to-functions": "workspace:^",
"@babel/helper-optimise-call-expression": "workspace:^",
"@babel/traverse": "workspace:^",
Expand Down
56 changes: 11 additions & 45 deletions packages/babel-helper-replace-supers/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
import type { HubInterface, NodePath, Scope } from "@babel/traverse";
import traverse from "@babel/traverse";
import memberExpressionToFunctions from "@babel/helper-member-expression-to-functions";
import type { HandlerState } from "@babel/helper-member-expression-to-functions";
import optimiseCall from "@babel/helper-optimise-call-expression";
import environmentVisitor from "@babel/helper-environment-visitor";
import {
VISITOR_KEYS,
assignmentExpression,
booleanLiteral,
callExpression,
cloneNode,
identifier,
memberExpression,
sequenceExpression,
staticBlock,
stringLiteral,
thisExpression,
} from "@babel/types";
import type * as t from "@babel/types";

// TODO (Babel 8): Don't export this.
export {
default as environmentVisitor,
skipAllButComputedKey,
} from "@babel/helper-environment-visitor";

/**
* Creates an expression which result is the proto of objectRef.
*
Expand All @@ -38,49 +44,9 @@ function getPrototypeOfExpression(objectRef, isStatic, file, isPrivateMethod) {
return callExpression(file.addHelper("getPrototypeOf"), [targetRef]);
}

export function skipAllButComputedKey(
path: NodePath<t.Method | t.ClassProperty | t.ClassPrivateProperty>,
) {
// If the path isn't computed, just skip everything.
// @ts-expect-error todo(flow->ts) check node type before cheking the property
if (!path.node.computed) {
path.skip();
return;
}

// So it's got a computed key. Make sure to skip every other key the
// traversal would visit.
const keys = VISITOR_KEYS[path.type];
for (const key of keys) {
if (key !== "key") path.skipKey(key);
}
}

// environmentVisitor should be used when traversing the whole class and not for specific class elements/methods.
// For perf reasons, the environmentVisitor will be traversed with `{ noScope: true }`, which means `path.scope` is undefined.
// Avoid using `path.scope` here
export const environmentVisitor = {
// todo (Babel 8): remove StaticBlock brand checks
[`${staticBlock ? "StaticBlock|" : ""}ClassPrivateProperty|TypeAnnotation`](
path: NodePath,
) {
path.skip();
},

Function(path: NodePath) {
// Methods will be handled by the Method visit
if (path.isMethod()) return;
// Arrow functions inherit their parent's environment
if (path.isArrowFunctionExpression()) return;
path.skip();
},

"Method|ClassProperty"(path: NodePath<t.Method | t.ClassProperty>) {
skipAllButComputedKey(path);
},
};

const visitor = traverse.visitors.merge([
const visitor = traverse.visitors.merge<
HandlerState<ReplaceState> & ReplaceState
>([
environmentVisitor,
{
Super(path, state) {
Expand Down
1 change: 1 addition & 0 deletions packages/babel-plugin-transform-classes/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"main": "./lib/index.js",
"dependencies": {
"@babel/helper-annotate-as-pure": "workspace:^",
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-function-name": "workspace:^",
"@babel/helper-optimise-call-expression": "workspace:^",
"@babel/helper-plugin-utils": "workspace:^",
Expand Down
5 changes: 2 additions & 3 deletions packages/babel-plugin-transform-classes/src/transformClass.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import type { NodePath, Visitor } from "@babel/traverse";
import nameFunction from "@babel/helper-function-name";
import ReplaceSupers, {
environmentVisitor,
} from "@babel/helper-replace-supers";
import ReplaceSupers from "@babel/helper-replace-supers";
import environmentVisitor from "@babel/helper-environment-visitor";
import optimiseCall from "@babel/helper-optimise-call-expression";
import { traverse, template, types as t } from "@babel/core";
import annotateAsPure from "@babel/helper-annotate-as-pure";
Expand Down
1 change: 1 addition & 0 deletions packages/babel-traverse/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"dependencies": {
"@babel/code-frame": "workspace:^",
"@babel/generator": "workspace:^",
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-function-name": "workspace:^",
"@babel/helper-hoist-variables": "workspace:^",
"@babel/helper-split-export-declaration": "workspace:^",
Expand Down

0 comments on commit 36a5ac4

Please sign in to comment.