From f3bd9cbcb8caf8a8172f3bcd23a3edf087e49941 Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Tue, 10 Mar 2015 13:04:02 +1100 Subject: [PATCH] use a different helper if a class contains class methods to avoid non-enumerability and delegation to es6.properties.computed transformer - fixes #984, closes #986 --- src/babel/transformation/file.js | 1 + .../transformation/helpers/define-map.js | 74 ++++++++----------- .../templates/create-computed-class.js | 18 +++++ .../transformers/es6/classes.js | 9 ++- src/babel/types/index.js | 19 ++++- .../es6-classes/computed-methods/actual.js | 5 ++ .../es6-classes/computed-methods/exec.js | 17 +++++ .../es6-classes/computed-methods/expected.js | 19 +++++ .../object-getter-memoization/expected.js | 14 ++-- 9 files changed, 122 insertions(+), 54 deletions(-) create mode 100644 src/babel/transformation/templates/create-computed-class.js create mode 100644 test/fixtures/transformation/es6-classes/computed-methods/actual.js create mode 100644 test/fixtures/transformation/es6-classes/computed-methods/exec.js create mode 100644 test/fixtures/transformation/es6-classes/computed-methods/expected.js diff --git a/src/babel/transformation/file.js b/src/babel/transformation/file.js index 6017cf8bfa..cfebe4a51a 100644 --- a/src/babel/transformation/file.js +++ b/src/babel/transformation/file.js @@ -50,6 +50,7 @@ export default class File { "inherits", "defaults", "create-class", + "create-computed-class", "apply-constructor", "tagged-template-literal", "tagged-template-literal-loose", diff --git a/src/babel/transformation/helpers/define-map.js b/src/babel/transformation/helpers/define-map.js index 02819ae6cd..068cb899bc 100644 --- a/src/babel/transformation/helpers/define-map.js +++ b/src/babel/transformation/helpers/define-map.js @@ -5,33 +5,40 @@ import has from "lodash/object/has"; import t from "../../types"; export function push(mutatorMap, key, kind, computed, value) { - var alias; + var alias = t.toKeyAlias({ computed }, key); - if (t.isIdentifier(key)) { - alias = key.name; - if (computed) alias = `computed:${alias}`; - } else if (t.isLiteral(key)) { - alias = String(key.value); - } else { - alias = JSON.stringify(traverse.removeProperties(t.cloneDeep(key))); - } - - var map; - if (has(mutatorMap, alias)) { - map = mutatorMap[alias]; - } else { - map = {}; - } + var map = {}; + if (has(mutatorMap, alias)) map = mutatorMap[alias]; mutatorMap[alias] = map; map._key = key; - if (computed) { - map._computed = true; - } + if (computed) map._computed = true; map[kind] = value; } +export function hasComputed(mutatorMap) { + for (var key in mutatorMap) { + if (mutatorMap[key]._computed) { + return true; + } + } + return false; +} + +export function toComputedObjectFromClass(obj) { + var objExpr = t.arrayExpression([]); + + for (var i = 0; i < obj.properties.length; i++) { + var prop = obj.properties[i]; + var val = prop.value; + val.properties.unshift(t.property("init", t.identifier("key"), t.toComputedKey(prop))); + objExpr.elements.push(val); + } + + return objExpr; +} + export function toClassObject(mutatorMap) { var objExpr = t.objectExpression([]); @@ -49,6 +56,7 @@ export function toClassObject(mutatorMap) { var prop = t.property("init", t.identifier(key), node); t.inheritsComments(prop, inheritNode); t.removeComments(inheritNode); + mapNode.properties.push(prop); }); @@ -59,35 +67,11 @@ export function toClassObject(mutatorMap) { } export function toDefineObject(mutatorMap) { - var objExpr = t.objectExpression([]); - each(mutatorMap, function (map) { - var mapNode = t.objectExpression([]); - - var propNode = t.property("init", map._key, mapNode, map._computed); - - if (map.value) { - map.writable = t.literal(true); - } - + if (map.value) map.writable = t.literal(true); map.configurable = t.literal(true); map.enumerable = t.literal(true); - - each(map, function (node, key) { - if (key[0] === "_") return; - - node = t.clone(node); - var inheritNode = node; - if (t.isMethodDefinition(node)) node = node.value; - - var prop = t.property("init", t.identifier(key), node); - t.inheritsComments(prop, inheritNode); - t.removeComments(inheritNode); - mapNode.properties.push(prop); - }); - - objExpr.properties.push(propNode); }); - return objExpr; + return toClassObject(mutatorMap); } diff --git a/src/babel/transformation/templates/create-computed-class.js b/src/babel/transformation/templates/create-computed-class.js new file mode 100644 index 0000000000..4748cde83f --- /dev/null +++ b/src/babel/transformation/templates/create-computed-class.js @@ -0,0 +1,18 @@ +(function() { + function defineProperties(target, rawProps) { + var props = {}; + for (var i = 0; i < rawProps.length; i ++) { + var prop = rawProps[i]; + prop.configurable = true; + if (prop.value) prop.writable = true; + props[prop.key] = prop; + } + Object.defineProperties(target, props); + } + + return function (Constructor, protoProps, staticProps) { + if (protoProps) defineProperties(Constructor.prototype, protoProps); + if (staticProps) defineProperties(Constructor, staticProps); + return Constructor; + }; +})() diff --git a/src/babel/transformation/transformers/es6/classes.js b/src/babel/transformation/transformers/es6/classes.js index d7b606099f..dd62afe344 100644 --- a/src/babel/transformation/transformers/es6/classes.js +++ b/src/babel/transformation/transformers/es6/classes.js @@ -217,6 +217,7 @@ class ClassTransformer { var instanceProps; var staticProps; + var classHelper = "create-class"; if (this.hasInstanceMutators) { instanceProps = defineMap.toClassObject(this.instanceMutatorMap); @@ -227,13 +228,19 @@ class ClassTransformer { } if (instanceProps || staticProps) { + if (defineMap.hasComputed(this.instanceMutatorMap) || defineMap.hasComputed(this.staticMutatorMap)) { + if (instanceProps) instanceProps = defineMap.toComputedObjectFromClass(instanceProps); + if (staticProps) staticProps = defineMap.toComputedObjectFromClass(staticProps); + classHelper = "create-computed-class"; + } + instanceProps ||= t.literal(null); var args = [this.classRef, instanceProps]; if (staticProps) args.push(staticProps); body.push(t.expressionStatement( - t.callExpression(this.file.addHelper("create-class"), args) + t.callExpression(this.file.addHelper(classHelper), args) )); } } diff --git a/src/babel/types/index.js b/src/babel/types/index.js index 58d115ae1b..a137fd367d 100644 --- a/src/babel/types/index.js +++ b/src/babel/types/index.js @@ -126,7 +126,7 @@ each(t.BUILDER_KEYS, function (keys, type) { * Description */ -t.toComputedKey = function (node: Object, key: Object): Object { +t.toComputedKey = function (node: Object, key: Object = node.key): Object { if (!node.computed) { if (t.isIdentifier(key)) key = t.literal(key.name); } @@ -701,6 +701,23 @@ t.getLastStatements = function (node: Object): Array { return nodes; }; +/** + * Description + */ + +t.toKeyAlias = function (node: Object, key: Object = node.key) { + var alias; + if (t.isIdentifier(key)) { + alias = key.name; + } else if (t.isLiteral(key)) { + alias = JSON.stringify(key.value); + } else { + alias = JSON.stringify(traverse.removeProperties(t.cloneDeep(key))); + } + if (node.computed) alias = `[${alias}]`; + return alias; +}; + /** * Description */ diff --git a/test/fixtures/transformation/es6-classes/computed-methods/actual.js b/test/fixtures/transformation/es6-classes/computed-methods/actual.js new file mode 100644 index 0000000000..86ad92a32a --- /dev/null +++ b/test/fixtures/transformation/es6-classes/computed-methods/actual.js @@ -0,0 +1,5 @@ +class Foo { + foo() {} + "foo"() {} + [bar]() {} +} diff --git a/test/fixtures/transformation/es6-classes/computed-methods/exec.js b/test/fixtures/transformation/es6-classes/computed-methods/exec.js new file mode 100644 index 0000000000..6b0063c339 --- /dev/null +++ b/test/fixtures/transformation/es6-classes/computed-methods/exec.js @@ -0,0 +1,17 @@ +const sym = Symbol(); + +class Foo { + [sym] () { + return 1; + } +} + +class Bar extends Foo { + [sym] () { + return super[sym]() + 2; + } +} + +let i = new Bar(); + +assert.equal(i[sym](), 3); diff --git a/test/fixtures/transformation/es6-classes/computed-methods/expected.js b/test/fixtures/transformation/es6-classes/computed-methods/expected.js new file mode 100644 index 0000000000..0b344aa3d2 --- /dev/null +++ b/test/fixtures/transformation/es6-classes/computed-methods/expected.js @@ -0,0 +1,19 @@ +"use strict"; + +var Foo = (function () { + function Foo() { + babelHelpers.classCallCheck(this, Foo); + } + + babelHelpers.createComputedClass(Foo, [{ + key: "foo", + value: function foo() {} + }, { + key: "foo", + value: function foo() {} + }, { + key: bar, + value: function () {} + }]); + return Foo; +})(); diff --git a/test/fixtures/transformation/playground/object-getter-memoization/expected.js b/test/fixtures/transformation/playground/object-getter-memoization/expected.js index cc9dbcb09c..214f9235f5 100644 --- a/test/fixtures/transformation/playground/object-getter-memoization/expected.js +++ b/test/fixtures/transformation/playground/object-getter-memoization/expected.js @@ -5,17 +5,17 @@ var Foo = (function () { babelHelpers.classCallCheck(this, Foo); } - babelHelpers.createClass(Foo, babelHelpers.defineProperty({ - bar: { - get: function () { - return babelHelpers.defineProperty(this, "bar", complex()).bar; - } + babelHelpers.createComputedClass(Foo, [{ + key: "bar", + get: function () { + return babelHelpers.defineProperty(this, "bar", complex()).bar; } - }, bar, { + }, { + key: bar, get: function () { return babelHelpers.defineProperty(this, bar, complex())[bar]; } - })); + }]); return Foo; })();