From 30b15de67c12713c058bcaf15974d79cefdc9458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Thu, 27 Dec 2018 11:30:55 +0100 Subject: [PATCH] [decorators] Set method names at compile time instead of at runtime In gh-8743 I used `Object.defineProperty` in the decorators helper to set the correct methods name: it was needed because methods were renamed to `value`. Apparently some browsers set `Function#name` as non-configurable: gh-9234, so it throws. This commit removes that fix from the helper and changes the transform so that it gives the correct name at compile time. It doesn't work with computed keys, but it should be ok because we already don't handle method names in classes and object methods. --- .../src/decorators.js | 30 ++++++++++++------- packages/babel-helpers/src/helpers.js | 4 --- .../computed-keys-same-ast/output.js | 8 ++--- .../computed-keys-same-value/output.js | 8 ++--- .../misc/method-name-not-shadow/exec.js | 13 ++++++++ .../misc/method-name-not-shadow/input.js | 8 +++++ .../misc/method-name-not-shadow/output.js | 22 ++++++++++++++ .../transformation/arguments/output.js | 4 +-- .../async-generator-method/output.js | 12 ++------ .../transformation/strict-directive/output.js | 8 ++--- 10 files changed, 73 insertions(+), 44 deletions(-) create mode 100644 packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/exec.js create mode 100644 packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/input.js create mode 100644 packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/output.js diff --git a/packages/babel-helper-create-class-features-plugin/src/decorators.js b/packages/babel-helper-create-class-features-plugin/src/decorators.js index 4b0df1aef49c..74fc7d059523 100644 --- a/packages/babel-helper-create-class-features-plugin/src/decorators.js +++ b/packages/babel-helper-create-class-features-plugin/src/decorators.js @@ -1,5 +1,6 @@ import { types as t, template } from "@babel/core"; import ReplaceSupers from "@babel/helper-replace-supers"; +import nameFunction from "@babel/helper-function-name"; export function hasOwnDecorators(node) { return !!(node.decorators && node.decorators.length); @@ -14,11 +15,13 @@ function prop(key, value) { return t.objectProperty(t.identifier(key), value); } -function value(body, params = [], async, generator) { - const method = t.objectMethod("method", t.identifier("value"), params, body); - method.async = !!async; - method.generator = !!generator; - return method; +function method(key, body) { + return t.objectMethod( + "method", + t.identifier(key), + [], + t.blockStatement(body), + ); } function takeDecorators(node) { @@ -74,13 +77,20 @@ function extractElementDescriptor(/* this: File, */ classRef, superRef, path) { prop("decorators", takeDecorators(node)), prop("static", node.static && t.booleanLiteral(true)), prop("key", getKey(node)), - isMethod - ? value(node.body, node.params, node.async, node.generator) - : node.value - ? value(template.ast`{ return ${node.value} }`) - : prop("value", scope.buildUndefinedNode()), ].filter(Boolean); + if (isMethod) { + const id = node.computed ? null : node.key; + t.toExpression(node); + properties.push(prop("value", nameFunction({ node, id, scope }) || node)); + } else if (node.value) { + properties.push( + method("value", template.statements.ast`return ${node.value}`), + ); + } else { + properties.push(prop("value", scope.buildUndefinedNode())); + } + path.remove(); return t.objectExpression(properties); diff --git a/packages/babel-helpers/src/helpers.js b/packages/babel-helpers/src/helpers.js index 4c3c896fcc48..4ba0f7458f6e 100644 --- a/packages/babel-helpers/src/helpers.js +++ b/packages/babel-helpers/src/helpers.js @@ -1246,10 +1246,6 @@ helpers.decorate = helper("7.1.5")` configurable: true, enumerable: false, }; - Object.defineProperty(def.value, "name", { - value: typeof key === "symbol" ? "" : key, - configurable: true, - }); } else if (def.kind === "get") { descriptor = { get: def.value, configurable: true, enumerable: false }; } else if (def.kind === "set") { diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-ast/output.js b/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-ast/output.js index a08d2e1db86d..99ce59522cb1 100644 --- a/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-ast/output.js +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-ast/output.js @@ -13,19 +13,15 @@ let Foo = babelHelpers.decorate([_ => desc = _], function (_initialize) { d: [{ kind: "method", key: getKey(), - - value() { + value: function () { return 1; } - }, { kind: "method", key: getKey(), - - value() { + value: function () { return 2; } - }] }; }); diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-value/output.js b/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-value/output.js index bffa9e17a1d9..a4f7d0e22920 100644 --- a/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-value/output.js +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/duplicated-keys/computed-keys-same-value/output.js @@ -13,19 +13,15 @@ let Foo = babelHelpers.decorate([_ => desc = _], function (_initialize) { d: [{ kind: "method", key: getKeyI(), - - value() { + value: function () { return 1; } - }, { kind: "method", key: getKeyJ(), - - value() { + value: function () { return 2; } - }] }; }); diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/exec.js b/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/exec.js new file mode 100644 index 000000000000..b9d3327d304d --- /dev/null +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/exec.js @@ -0,0 +1,13 @@ +function decorator() {} + +var method = 1; + +@decorator +class Foo { + method() { + return method; + } +} + +expect(new Foo().method()).toBe(1); +expect(Foo.prototype.method.name).toBe("method"); diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/input.js b/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/input.js new file mode 100644 index 000000000000..ce5f357574c3 --- /dev/null +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/input.js @@ -0,0 +1,8 @@ +var method = 1; + +@decorator +class Foo { + method() { + return method; + } +} diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/output.js b/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/output.js new file mode 100644 index 000000000000..df5e4ae17032 --- /dev/null +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/misc/method-name-not-shadow/output.js @@ -0,0 +1,22 @@ +var _method = 1; +let Foo = babelHelpers.decorate([decorator], function (_initialize) { + "use strict"; + + class Foo { + constructor() { + _initialize(this); + } + + } + + return { + F: Foo, + d: [{ + kind: "method", + key: "method", + value: function method() { + return _method; + } + }] + }; +}); diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/arguments/output.js b/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/arguments/output.js index d8a47bf3a19c..5be9a05bed5b 100644 --- a/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/arguments/output.js +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/arguments/output.js @@ -14,9 +14,7 @@ let A = babelHelpers.decorate([dec(a, b, ...c)], function (_initialize) { kind: "method", decorators: [dec(a, b, ...c)], key: "method", - - value() {} - + value: function method() {} }] }; }); diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/async-generator-method/output.js b/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/async-generator-method/output.js index 75ce76d3f1d7..973d53b8620a 100644 --- a/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/async-generator-method/output.js +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/async-generator-method/output.js @@ -13,21 +13,15 @@ let Foo = babelHelpers.decorate([decorator], function (_initialize) { d: [{ kind: "method", key: "f1", - - async value() {} - + value: async function f1() {} }, { kind: "method", key: "f2", - - *value() {} - + value: function* f2() {} }, { kind: "method", key: "f3", - - async *value() {} - + value: async function* f3() {} }] }; }); diff --git a/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/strict-directive/output.js b/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/strict-directive/output.js index 65f4fbb073ea..e1a4d147adbf 100644 --- a/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/strict-directive/output.js +++ b/packages/babel-plugin-proposal-decorators/test/fixtures/transformation/strict-directive/output.js @@ -14,9 +14,7 @@ d: [{ kind: "method", key: "method", - - value() {} - + value: function method() {} }] }; }); @@ -38,9 +36,7 @@ d: [{ kind: "method", key: "method", - - value() {} - + value: function method() {} }] }; });