From 72b5d58560930c5c1562b165927812dc56132edb Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 6 Dec 2024 02:20:57 +0000 Subject: [PATCH] fix(transformer/class-properties): create temp var for `this` in computed key (#7686) `this` in computed key needs a temp var, otherwise once moved into constructor it refers to the wrong `this`. --- .../src/es2022/class_properties/class.rs | 13 ++++++-- .../snapshots/oxc.snap.md | 3 +- .../test/fixtures/options.json | 5 ++++ .../fixtures/this-in-computed-key/input.js | 22 ++++++++++++++ .../fixtures/this-in-computed-key/output.js | 30 +++++++++++++++++++ 5 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/options.json create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/input.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/output.js diff --git a/crates/oxc_transformer/src/es2022/class_properties/class.rs b/crates/oxc_transformer/src/es2022/class_properties/class.rs index 2f3a52bd3652b..597d4afd5fbf6 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class.rs @@ -725,6 +725,16 @@ 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(_) @@ -732,8 +742,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { | 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 diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md index 2c963e3f46def..28790404a475c 100644 --- a/tasks/transform_conformance/snapshots/oxc.snap.md +++ b/tasks/transform_conformance/snapshots/oxc.snap.md @@ -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 diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/options.json b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/options.json new file mode 100644 index 0000000000000..6f80120055293 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/options.json @@ -0,0 +1,5 @@ +{ + "plugins": [ + "transform-class-properties" + ] +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/input.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/input.js new file mode 100644 index 0000000000000..c04fae047aa6d --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/input.js @@ -0,0 +1,22 @@ +function createClassDeclaration() { + class C { + [this] = 1; + [this + 'bar'] = 2; + } + return C; +} + +function createClassExpression() { + return class { + [this] = 3; + [this + 'bar'] = 4; + }; +} + +const C = createClassDeclaration.call("foo"); +expect(new C().foo).toBe(1); +expect(new C().foobar).toBe(2); + +const D = createClassExpression.call("foo"); +expect(new D().foo).toBe(3); +expect(new D().foobar).toBe(4); diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/output.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/output.js new file mode 100644 index 0000000000000..9429054927cc4 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/this-in-computed-key/output.js @@ -0,0 +1,30 @@ +function createClassDeclaration() { + let _this, _ref; + _this = this; + _ref = this + "bar"; + class C { + constructor() { + babelHelpers.defineProperty(this, _this, 1); + babelHelpers.defineProperty(this, _ref, 2); + } + } + return C; +} + +function createClassExpression() { + let _this2, _ref2; + return _this2 = this, _ref2 = this + "bar", class { + constructor() { + babelHelpers.defineProperty(this, _this2, 3); + babelHelpers.defineProperty(this, _ref2, 4); + } + }; +} + +const C = createClassDeclaration.call("foo"); +expect(new C().foo).toBe(1); +expect(new C().foobar).toBe(2); + +const D = createClassExpression.call("foo"); +expect(new D().foo).toBe(3); +expect(new D().foobar).toBe(4);