Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

environmentVisitor should skip decorator expressions #14371

Merged
merged 12 commits into from May 21, 2022
6 changes: 2 additions & 4 deletions packages/babel-helper-environment-visitor/package.json
Expand Up @@ -17,11 +17,9 @@
".": "./lib/index.js",
"./package.json": "./package.json"
},
"dependencies": {
"@babel/types": "workspace:^"
},
"devDependencies": {
"@babel/traverse": "workspace:^"
"@babel/traverse": "workspace:^",
"@babel/types": "workspace:^"
},
"engines": {
"node": ">=6.9.0"
Expand Down
62 changes: 40 additions & 22 deletions packages/babel-helper-environment-visitor/src/index.ts
@@ -1,38 +1,56 @@
import type { NodePath } from "@babel/traverse";
import { VISITOR_KEYS, staticBlock } from "@babel/types";
import type { NodePath, Visitor } from "@babel/traverse";
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;
path.skip();
if (path.node.computed) {
// requeue the computed key
path.context.maybeQueue(path.get("key"));
}
}

// 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);
export function requeueComputedKeyAndDecorators(
path: NodePath<t.Method | t.Property>,
) {
const { context, node } = path;
//@ts-ignore ClassPrivateProperty does not have computed
if (node.computed) {
// requeue the computed key
context.maybeQueue(path.get("key"));
}
if (node.decorators) {
for (const decorator of path.get("decorators")) {
// requeue the decorators
context.maybeQueue(decorator);
}
}
}

// 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);
const visitor: Visitor = {
FunctionParent(path) {
if (path.isArrowFunctionExpression()) {
// arrows are not skipped because they inherit the context.
return;
} else {
path.skip();
if (path.isMethod()) {
requeueComputedKeyAndDecorators(path);
}
}
},
Property(path) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The visitor now hooks on Property alias so that it supports older @babel/types without ClassAccessorProperty definitions.

if (path.isObjectProperty()) {
return;
}
path.skip();
requeueComputedKeyAndDecorators(path);
},
};

export default visitor;
4 changes: 3 additions & 1 deletion packages/babel-helper-replace-supers/src/index.ts
Expand Up @@ -57,7 +57,9 @@ const visitor = traverse.visitors.merge<
},
]);

const unshadowSuperBindingVisitor = traverse.visitors.merge([
const unshadowSuperBindingVisitor = traverse.visitors.merge<{
refName: string;
}>([
environmentVisitor,
{
Scopable(path, { refName }) {
Expand Down
@@ -1,3 +1,8 @@
{
"plugins": ["proposal-class-properties", "transform-classes"]
"plugins": [
["proposal-decorators", { "version": "2021-12" }],
"proposal-class-static-block",
"proposal-class-properties",
"transform-classes"
]
}
@@ -0,0 +1,18 @@
"use strict";
class Hello {
constructor() {
return () => () => "hello";
}
}

class Outer extends Hello {
constructor() {
class Inner {
@(super()) hello;
}

return new Inner();
}
}

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,18 @@
"use strict";
class Hello {
constructor() {
return () => () => "hello";
}
}

class Outer extends Hello {
constructor() {
class Inner {
@(super()) hello;
}

return new Inner();
}
}

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,31 @@
"use strict";

let Hello = /*#__PURE__*/babelHelpers.createClass(function Hello() {
babelHelpers.classCallCheck(this, Hello);
return () => () => "hello";
});

let Outer = /*#__PURE__*/function (_Hello) {
babelHelpers.inherits(Outer, _Hello);

var _super = babelHelpers.createSuper(Outer);

function Outer() {
var _dec, _init_hello;

var _this;

babelHelpers.classCallCheck(this, Outer);
_dec = _this = _super.call(this);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this line is changed after rebasing. Maybe a generator update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#14398 probably

let Inner = /*#__PURE__*/babelHelpers.createClass(function Inner() {
babelHelpers.classCallCheck(this, Inner);
babelHelpers.defineProperty(this, "hello", _init_hello(this));
});
[_init_hello] = babelHelpers.applyDecs(Inner, [[_dec, 0, "hello"]], []);
return babelHelpers.possibleConstructorReturn(_this, new Inner());
}

return babelHelpers.createClass(Outer);
}(Hello);

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,19 @@
"use strict";
class Hello {
toString() {
return 'hello';
}
}

class Outer extends Hello {
constructor() {
super();
class Inner {
accessor [super.toString()] = 'hello';
}

return new Inner();
}
}

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,19 @@
"use strict";
class Hello {
toString() {
return 'hello';
}
}

class Outer extends Hello {
constructor() {
super();
class Inner {
accessor [super.toString()] = 'hello';
}

return new Inner();
}
}

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,8 @@
{
"plugins": [
["proposal-decorators", { "version": "2021-12" }],
"proposal-class-static-block",
"proposal-class-properties",
"transform-classes"
]
}
@@ -0,0 +1,64 @@
"use strict";

let Hello = /*#__PURE__*/function () {
function Hello() {
babelHelpers.classCallCheck(this, Hello);
}

babelHelpers.createClass(Hello, [{
key: "toString",
value: function toString() {
return 'hello';
}
}]);
return Hello;
}();

let Outer = /*#__PURE__*/function (_Hello) {
babelHelpers.inherits(Outer, _Hello);

var _super = babelHelpers.createSuper(Outer);

function Outer() {
let _babelHelpers$get$cal, _babelHelpers$get$cal2;

var _thisSuper, _this;

babelHelpers.classCallCheck(this, Outer);
_this = _super.call(this);

var _A = /*#__PURE__*/new WeakMap();

_babelHelpers$get$cal = babelHelpers.get((_thisSuper = babelHelpers.assertThisInitialized(_this), babelHelpers.getPrototypeOf(Outer.prototype)), "toString", _thisSuper).call(_thisSuper);
_babelHelpers$get$cal2 = babelHelpers.get((_thisSuper = babelHelpers.assertThisInitialized(_this), babelHelpers.getPrototypeOf(Outer.prototype)), "toString", _thisSuper).call(_thisSuper);

let Inner = /*#__PURE__*/function () {
function Inner() {
babelHelpers.classCallCheck(this, Inner);
babelHelpers.classPrivateFieldInitSpec(this, _A, {
writable: true,
value: 'hello'
});
}

babelHelpers.createClass(Inner, [{
key: _babelHelpers$get$cal,
get: function () {
return babelHelpers.classPrivateFieldGet(this, _A);
}
}, {
key: _babelHelpers$get$cal2,
set: function (v) {
babelHelpers.classPrivateFieldSet(this, _A, v);
}
}]);
return Inner;
}();

return babelHelpers.possibleConstructorReturn(_this, new Inner());
}

return babelHelpers.createClass(Outer);
}(Hello);

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,17 @@
"use strict";
class Hello {
dec() { return () => "hello" }
}

class Outer extends Hello {
constructor() {
super();
class Inner {
@(super.dec) hello;
}

return new Inner();
}
}

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,17 @@
"use strict";
class Hello {
dec() { return () => "hello" }
}

class Outer extends Hello {
constructor() {
super();
class Inner {
@(super.dec) hello;
}

return new Inner();
}
}

expect(new Outer().hello).toBe('hello');
@@ -0,0 +1,41 @@
"use strict";

let Hello = /*#__PURE__*/function () {
function Hello() {
babelHelpers.classCallCheck(this, Hello);
}

babelHelpers.createClass(Hello, [{
key: "dec",
value: function dec() {
return () => "hello";
}
}]);
return Hello;
}();

let Outer = /*#__PURE__*/function (_Hello) {
babelHelpers.inherits(Outer, _Hello);

var _super = babelHelpers.createSuper(Outer);

function Outer() {
var _dec, _init_hello;

var _thisSuper, _this;

babelHelpers.classCallCheck(this, Outer);
_this = _super.call(this);
_dec = babelHelpers.get((_thisSuper = babelHelpers.assertThisInitialized(_this), babelHelpers.getPrototypeOf(Outer.prototype)), "dec", _thisSuper);
let Inner = /*#__PURE__*/babelHelpers.createClass(function Inner() {
babelHelpers.classCallCheck(this, Inner);
babelHelpers.defineProperty(this, "hello", _init_hello(this));
});
[_init_hello] = babelHelpers.applyDecs(Inner, [[_dec, 0, "hello"]], []);
return babelHelpers.possibleConstructorReturn(_this, new Inner());
}

return babelHelpers.createClass(Outer);
}(Hello);

expect(new Outer().hello).toBe('hello');
Expand Up @@ -502,7 +502,7 @@ function transformClass(
if (element.node.decorators && element.node.decorators.length > 0) {
hasElementDecorators = true;
} else if (element.node.type === "ClassAccessorProperty") {
const { key, value, static: isStatic } = element.node;
const { key, value, static: isStatic, computed } = element.node;

const newId = generateClassPrivateUid();

Expand All @@ -511,7 +511,7 @@ function transformClass(
const newField = generateClassProperty(newId, valueNode, isStatic);

const [newPath] = element.replaceWith(newField);
addProxyAccessorsFor(newPath, key, newId, element.node.computed);
addProxyAccessorsFor(newPath, key, newId, computed);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug fix, the element.node.computed is of the replacement node, which is always false.

}
}

Expand Down