From 5f285c1034acebf9303018a541831da143823cc4 Mon Sep 17 00:00:00 2001 From: Qantas94Heavy Date: Fri, 13 Oct 2017 21:19:48 +1100 Subject: [PATCH] Evaluate computed class props only once (#6240) (#6466) Previously, computed class properties would be evaluated every time a new instance of the class was created. This means the property name may have changed between different instances, as well as potential side effects. This commit fixes this by storing the computed value in a separate variable. --- .../src/index.js | 14 ++++++++++++++ .../general/instance-computed/actual.js | 13 +++++++++++++ .../general/instance-computed/exec.js | 13 +++++++++++++ .../general/instance-computed/expected.js | 19 +++++++++++++++++++ .../loose/instance-computed/actual.js | 14 ++++++++++++-- .../fixtures/loose/instance-computed/exec.js | 13 +++++++++++++ .../loose/instance-computed/expected.js | 18 ++++++++++++++---- 7 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/actual.js create mode 100644 packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/exec.js create mode 100644 packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/expected.js create mode 100644 packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/exec.js diff --git a/packages/babel-plugin-transform-class-properties/src/index.js b/packages/babel-plugin-transform-class-properties/src/index.js index 7fd17a62ea..cd2a7633cb 100644 --- a/packages/babel-plugin-transform-class-properties/src/index.js +++ b/packages/babel-plugin-transform-class-properties/src/index.js @@ -96,6 +96,20 @@ export default function({ types: t }, options) { if (isStatic) { nodes.push(buildClassProperty(ref, propNode, path.scope)); } else { + // Make sure computed property names are only evaluated once (upon + // class definition). + if (propNode.computed) { + const ident = path.scope.generateUidIdentifierBasedOnNode( + propNode.key, + ); + nodes.push( + t.variableDeclaration("var", [ + t.variableDeclarator(ident, propNode.key), + ]), + ); + propNode.key = ident; + } + instanceBody.push( buildClassProperty(t.thisExpression(), propNode, path.scope), ); diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/actual.js b/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/actual.js new file mode 100644 index 0000000000..9a45b97c5f --- /dev/null +++ b/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/actual.js @@ -0,0 +1,13 @@ +function test(x) { + class F { + [x] = 1; + constructor() {} + } + + x = 'deadbeef'; + assert.strictEqual(new F().foo, 1); + x = 'wrong'; + assert.strictEqual(new F().foo, 1); +} + +test('foo'); diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/exec.js b/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/exec.js new file mode 100644 index 0000000000..9a45b97c5f --- /dev/null +++ b/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/exec.js @@ -0,0 +1,13 @@ +function test(x) { + class F { + [x] = 1; + constructor() {} + } + + x = 'deadbeef'; + assert.strictEqual(new F().foo, 1); + x = 'wrong'; + assert.strictEqual(new F().foo, 1); +} + +test('foo'); diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/expected.js b/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/expected.js new file mode 100644 index 0000000000..9c7c3d90a3 --- /dev/null +++ b/packages/babel-plugin-transform-class-properties/test/fixtures/general/instance-computed/expected.js @@ -0,0 +1,19 @@ +function test(x) { + var F = function F() { + babelHelpers.classCallCheck(this, F); + Object.defineProperty(this, _x, { + configurable: true, + enumerable: true, + writable: true, + value: 1 + }); + }; + + var _x = x; + x = 'deadbeef'; + assert.strictEqual(new F().foo, 1); + x = 'wrong'; + assert.strictEqual(new F().foo, 1); +} + +test('foo'); diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/actual.js b/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/actual.js index 5ad5e1f561..9a45b97c5f 100644 --- a/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/actual.js +++ b/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/actual.js @@ -1,3 +1,13 @@ -class Foo { - [bar] = "foo"; +function test(x) { + class F { + [x] = 1; + constructor() {} + } + + x = 'deadbeef'; + assert.strictEqual(new F().foo, 1); + x = 'wrong'; + assert.strictEqual(new F().foo, 1); } + +test('foo'); diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/exec.js b/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/exec.js new file mode 100644 index 0000000000..9a45b97c5f --- /dev/null +++ b/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/exec.js @@ -0,0 +1,13 @@ +function test(x) { + class F { + [x] = 1; + constructor() {} + } + + x = 'deadbeef'; + assert.strictEqual(new F().foo, 1); + x = 'wrong'; + assert.strictEqual(new F().foo, 1); +} + +test('foo'); diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/expected.js b/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/expected.js index 913f3dde46..789f92f9c1 100644 --- a/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/expected.js +++ b/packages/babel-plugin-transform-class-properties/test/fixtures/loose/instance-computed/expected.js @@ -1,4 +1,14 @@ -var Foo = function Foo() { - babelHelpers.classCallCheck(this, Foo); - this[bar] = "foo"; -}; +function test(x) { + var F = function F() { + babelHelpers.classCallCheck(this, F); + this[_x] = 1; + }; + + var _x = x; + x = 'deadbeef'; + assert.strictEqual(new F().foo, 1); + x = 'wrong'; + assert.strictEqual(new F().foo, 1); +} + +test('foo');