Skip to content

Commit

Permalink
fix(transformer/class-properties): create temp var for this in comp…
Browse files Browse the repository at this point in the history
…uted key
  • Loading branch information
overlookmotel committed Dec 5, 2024
1 parent 8883968 commit 67fd742
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 3 deletions.
13 changes: 11 additions & 2 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,15 +725,24 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// Bound vars and literals do not need temp var - return unchanged.
// e.g. `let x = 'x'; class C { [x] = 1; }` or `class C { ['x'] = 1; }`
//
// `this` does not have side effects, but it needs a temp var anyway, because `this` in computed
// key and `this` within class constructor resolve to different `this` bindings.
// So we need to create a temp var outside of the class to get the correct `this`.
// `class C { [this] = 1; }`
// -> `let _this; _this = this; class C { constructor() { this[_this] = 1; } }`
//
// TODO(improve-on-babel): Can avoid the temp var if key is for a static prop/method,
// as in that case the usage of `this` stays outside the class.
//
// TODO: Do fuller analysis to detect expressions which cannot have side effects e.g. `'x' + 'y'`.
let cannot_have_side_effects = match &key {
Expression::BooleanLiteral(_)
| Expression::NullLiteral(_)
| Expression::NumericLiteral(_)
| Expression::BigIntLiteral(_)
| Expression::RegExpLiteral(_)
| Expression::StringLiteral(_)
| Expression::ThisExpression(_) => true,
| Expression::StringLiteral(_) => true,
Expression::Identifier(ident) => {
// Cannot have side effects if is bound.
// Additional check that the var is not mutated is required for cases like
Expand Down
3 changes: 2 additions & 1 deletion tasks/transform_conformance/snapshots/oxc.snap.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
commit: 54a8389f

Passed: 93/104
Passed: 94/105

# All Passed:
* babel-plugin-transform-class-properties
* babel-plugin-transform-class-static-block
* babel-plugin-transform-nullish-coalescing-operator
* babel-plugin-transform-optional-catch-binding
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"plugins": [
"transform-class-properties"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
function createClassDeclarations() {
class C {
[this] = 1;
[this + 'bar'] = 2;
}

class D {
[this]() { return 3; }
[this + 'bar']() { return 4; }
[this + 'qux'] = 5;
}

return [C, D];
}

function createClassExpressions() {
return [
class C {
[this] = 1;
[this + 'bar'] = 2;
},
class D {
[this]() { return 3; }
[this + 'bar']() { return 4; }
[this + 'qux'] = 5;
}
];
}

function testClasses(create) {
const [C, D] = create.call("foo");

expect(new C().foo).toBe(1);
expect(new C().foobar).toBe(2);

expect(new D().foo()).toBe(3);
expect(new D().foobar()).toBe(4);
expect(new D().fooqux).toBe(5);
}

testClasses(createClassDeclarations);
testClasses(createClassExpressions);
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
function createClassDeclarations() {
let _this, _ref, _this2, _ref2, _ref3;
_this = this;
_ref = this + "bar";
class C {
constructor() {
babelHelpers.defineProperty(this, _this, 1);
babelHelpers.defineProperty(this, _ref, 2);
}
}

_this2 = this;
_ref2 = this + "bar";
_ref3 = this + "qux";
class D {
constructor() {
babelHelpers.defineProperty(this, _ref3, 5);
}
[_this2]() {
return 3;
}
[_ref2]() {
return 4;
}
}
return [C, D];
}

function createClassExpressions() {
let _this3, _ref4, _this4, _ref5, _ref6;
return [(_this3 = this, _ref4 = this + "bar", class C {
constructor() {
babelHelpers.defineProperty(this, _this3, 1);
babelHelpers.defineProperty(this, _ref4, 2);
}
}), (_this4 = this, _ref5 = this + "bar", _ref6 = this + "qux", class D {
constructor() {
babelHelpers.defineProperty(this, _ref6, 5);
}
[_this4]() {
return 3;
}
[_ref5]() {
return 4;
}
})];
}

function testClasses(create) {
const [C, D] = create.call("foo");
expect(new C().foo).toBe(1);
expect(new C().foobar).toBe(2);
expect(new D().foo()).toBe(3);
expect(new D().foobar()).toBe(4);
expect(new D().fooqux).toBe(5);
}

testClasses(createClassDeclarations);
testClasses(createClassExpressions);

0 comments on commit 67fd742

Please sign in to comment.