Always ensure Reflect methods exist, except for ones that require des…

…criptor support. Remove usage of Object.create in tests where possible.
paulmillr · Jan 29, 2015 · dc170f0 · dc170f0
1 parent 569676d
commit dc170f0
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 84 deletions.
diff --git a/es6-shim.js b/es6-shim.js
@@ -2237,6 +2237,68 @@
     };
   };
 
+  // Some Reflect methods are basically the same as
+  // those on the Object global, except that a TypeError is thrown if
+  // target isn't an object. As well as returning a boolean indicating
+  // the success of the operation.
+  defineProperties(globals.Reflect, {
+    // Apply method in a functional form.
+    apply: ES.Call,
+
+    // New operator in a functional form.
+    construct: function construct(constructor, args) {
+      if (!ES.IsCallable(constructor)) {
+        throw new TypeError('First argument must be callable.');
+      }
+
+      return ES.Construct(constructor, args);
+    },
+
+    // When deleting a non-existent or configurable property,
+    // true is returned.
+    // When attempting to delete a non-configurable property,
+    // it will return false.
+    deleteProperty: throwUnlessTargetIsObject(function deleteProperty(target, key) {
+      if (supportsDescriptors) {
+        var desc = Object.getOwnPropertyDescriptor(target, key);
+
+        if (desc && !desc.configurable) {
+          return false;
+        }
+      }
+
+      // Will return true.
+      return delete target[key];
+    }),
+
+    enumerate: throwUnlessTargetIsObject(function enumerate(target) {
+      return new ObjectIterator(target, 'key');
+    }),
+
+    has: throwUnlessTargetIsObject(function has(target, key) {
+      return key in target;
+    }),
+
+    isExtensible: throwUnlessTargetIsObject(Object.isExtensible),
+
+    // Basically the result of calling the internal [[OwnPropertyKeys]].
+    // Concatenating propertyNames and propertySymbols should do the trick.
+    // This should continue to work together with a Symbol shim
+    // which overrides Object.getOwnPropertyNames and implements
+    // Object.getOwnPropertySymbols.
+    ownKeys: throwUnlessTargetIsObject(function ownKeys(target) {
+      var keys = Object.getOwnPropertyNames(target);
+
+      if (ES.IsCallable(Object.getOwnPropertySymbols)) {
+        keys.push.apply(keys, Object.getOwnPropertySymbols(target));
+      }
+
+      return keys;
+    }),
+
+    preventExtensions: wrapObjectFunction(Object.preventExtensions)
+  });
+
   if (supportsDescriptors) {
     var internal_get = function get(target, key, receiver) {
       var desc = Object.getOwnPropertyDescriptor(target, key);
@@ -2313,44 +2375,10 @@
       return false;
     };
 
-    // Some Reflect methods are basically the same as
-    // those on the Object global, except that a TypeError is thrown if
-    // target isn't an object. As well as returning a boolean indicating
-    // the success of the operation.
     defineProperties(globals.Reflect, {
-
-      // Apply method in a functional form.
-      apply: ES.Call,
-
-      // New operator in a functional form.
-      construct: function construct(constructor, args) {
-        if (!ES.IsCallable(constructor)) {
-          throw new TypeError('First argument must be callable.');
-        }
-
-        return ES.Construct(constructor, args);
-      },
-
       defineProperty: wrapObjectFunction(Object.defineProperty),
 
-      // When deleting a non-existant or configurable property,
-      // true is returned.
-      // When attempting to delete a non-configurable property,
-      // it will return false.
-      deleteProperty: throwUnlessTargetIsObject(function deleteProperty(target, key) {
-        var desc = Object.getOwnPropertyDescriptor(target, key);
-
-        if (desc && !desc.configurable) {
-          return false;
-        }
-
-        // Will return true.
-        return delete target[key];
-      }),
-
-      enumerate: throwUnlessTargetIsObject(function enumerate(target) {
-        return new ObjectIterator(target, 'key');
-      }),
+      getOwnPropertyDescriptor: throwUnlessTargetIsObject(Object.getOwnPropertyDescriptor),
 
       // Syntax in a functional form.
       get: throwUnlessTargetIsObject(function get(target, key) {
@@ -2359,31 +2387,6 @@
         return internal_get(target, key, receiver);
       }),
 
-      getOwnPropertyDescriptor: throwUnlessTargetIsObject(Object.getOwnPropertyDescriptor),
-
-      has: throwUnlessTargetIsObject(function has(target, key) {
-        return key in target;
-      }),
-
-      isExtensible: throwUnlessTargetIsObject(Object.isExtensible),
-
-      // Basically the result of calling the internal [[OwnPropertyKeys]].
-      // Concatenating propertyNames and propertySymbols should do the trick.
-      // This should continue to work together with a Symbol shim
-      // which overrides Object.getOwnPropertyNames and implements
-      // Object.getOwnPropertySymbols.
-      ownKeys: throwUnlessTargetIsObject(function ownKeys(target) {
-        var keys = Object.getOwnPropertyNames(target);
-
-        if (ES.IsCallable(Object.getOwnPropertySymbols)) {
-          keys.push.apply(keys, Object.getOwnPropertySymbols(target));
-        }
-
-        return keys;
-      }),
-
-      preventExtensions: wrapObjectFunction(Object.preventExtensions),
-
       set: throwUnlessTargetIsObject(function set(target, key, value) {
         var receiver = arguments.length > 3 ? arguments[3] : target;
 

diff --git a/test/reflect.js b/test/reflect.js
@@ -1,9 +1,14 @@
-/*global describe, it, expect, require, Reflect */
+/*global describe, it, xit, expect, require, Reflect */
 
-var ifSymbolsIt = typeof Symbol === 'function' ? it : xit;
+/*jshint notypeof: true */
+var hasSymbols = typeof Symbol === 'function' && typeof Symbol() === 'symbol';
+/*jshint notypeof: false */
+var ifSymbolsIt = hasSymbols ? it : xit;
 var describeIfGetProto = Object.getPrototypeOf ? describe : xdescribe;
 var describeIfSetProto = Object.setPrototypeOf ? describe : xdescribe;
 var describeIfES5 = Object.defineProperty ? describe : xdescribe;
+var describeIfGetProto = Object.getPrototypeOf ? describe : xdescribe;
+var ifES5It = Object.defineProperty ? it : xit;
 var ifFreezeIt = typeof Object.freeze === 'function' ? it : xit;
 
 // Reflect requires defineProperty
@@ -55,7 +60,7 @@ describe('Reflect', function () {
     expect(exported.Reflect).to.equal(Reflect);
   });
 
-  describeIfES5('Reflect.apply()', function () {
+  describe('Reflect.apply()', function () {
     it('is a function', function () {
       expect(typeof Reflect.apply).to.equal('function');
     });
@@ -87,7 +92,7 @@ describe('Reflect', function () {
     });
   });
 
-  describeIfES5('Reflect.construct()', function () {
+  describe('Reflect.construct()', function () {
     it('is a function', function () {
       expect(typeof Reflect.construct).to.equal('function');
     });
@@ -169,7 +174,7 @@ describe('Reflect', function () {
     });
   });
 
-  describeIfES5('Reflect.deleteProperty()', function () {
+  describe('Reflect.deleteProperty()', function () {
     it('is a function', function () {
       expect(typeof Reflect.deleteProperty).to.equal('function');
     });
@@ -180,7 +185,7 @@ describe('Reflect', function () {
       });
     });
 
-    it('returns true for success and false for failure', function () {
+    ifES5It('returns true for success and false for failure', function () {
       var o = { a: 1 };
 
       Object.defineProperty(o, 'b', { value: 2 });
@@ -208,12 +213,12 @@ describe('Reflect', function () {
     });
   });
 
-  describeIfES5('Reflect.enumerate()', function () {
+  describe('Reflect.enumerate()', function () {
     it('is a function', function () {
       expect(typeof Reflect.enumerate).to.equal('function');
     });
 
-    it('only includes enumerable properties', function () {
+    ifES5It('only includes enumerable properties', function () {
       var a = Object.create(null, {
         // Non-enumerable per default.
         a: { value: 1 }
@@ -224,18 +229,23 @@ describe('Reflect', function () {
       var iter = Reflect.enumerate(a);
 
       /*jshint notypeof: true */
-      if (typeof Symbol === 'function' && typeof Symbol.iterator === 'symbol') {
+      if (hasSymbols && typeof Symbol.iterator === 'symbol') {
         expect(Symbol.iterator in iter).to.equal(true);
       }
 
       expect(Array.from(iter)).to.deep.equal(['b']);
     });
 
     it('includes all enumerable properties of prototypes', function () {
-      var a = { prop: true };
-      var b = Object.create(a);
+      var Parent = function () {};
+      Parent.prototype.prop = true;
+
+      var Child = function () {};
+      Child.prototype = new Parent();
+      Child.prototype.childProp = true;
+      var child = new Child();
 
-      expect(Array.from(Reflect.enumerate(b))).to.deep.equal(['prop']);
+      expect(Array.from(Reflect.enumerate(child)).sort()).to.deep.equal(['prop', 'childProp'].sort());
     });
 
     it('yields keys determined at first next() call', function () {
@@ -401,7 +411,7 @@ describe('Reflect', function () {
     });
   });
 
-  describeIfES5('Reflect.has()', function () {
+  describe('Reflect.has()', function () {
     it('is a function', function () {
       expect(typeof Reflect.has).to.equal('function');
     });
@@ -413,7 +423,7 @@ describe('Reflect', function () {
     });
 
     it('will detect own properties', function () {
-      var target = Object.create(null);
+      var target = Object.create ? Object.create(null) : {};
 
       expect(Reflect.has(target, 'prop')).to.equal(false);
 
@@ -423,28 +433,35 @@ describe('Reflect', function () {
       delete target.prop;
       expect(Reflect.has(target, 'prop')).to.equal(false);
 
+      expect(Reflect.has(Reflect.has, 'length')).to.equal(true);
+    });
+
+    ifES5It('will detect an own accessor property', function () {
+      var target = Object.create(null);
       Object.defineProperty(target, 'accessor', {
         set: function () {}
       });
 
       expect(Reflect.has(target, 'accessor')).to.equal(true);
-
-      expect(Reflect.has(Reflect.has, 'length')).to.equal(true);
     });
 
     it('will search the prototype chain', function () {
-      var intermediate = Object.create(object),
-      target = Object.create(intermediate);
+      var Parent = function () {};
+      Parent.prototype.someProperty = undefined;
+
+      var Child = function () {};
+      Child.prototype = new Parent();
 
-      intermediate.some_property = undefined;
+      var target = new Child();
+      target.bool = true;
 
       expect(Reflect.has(target, 'bool')).to.equal(true);
-      expect(Reflect.has(target, 'some_property')).to.equal(true);
-      expect(Reflect.has(target, 'undefined_property')).to.equal(false);
+      expect(Reflect.has(target, 'someProperty')).to.equal(true);
+      expect(Reflect.has(target, 'undefinedProperty')).to.equal(false);
     });
   });
 
-  describeIfES5('Reflect.isExtensible()', function () {
+  describe('Reflect.isExtensible()', function () {
     it('is a function', function () {
       expect(typeof Reflect.isExtensible).to.equal('function');
     });
@@ -461,7 +478,7 @@ describe('Reflect', function () {
     });
   });
 
-  describeIfES5('Reflect.ownKeys()', function () {
+  describe('Reflect.ownKeys()', function () {
     it('is a function', function () {
       expect(typeof Reflect.ownKeys).to.equal('function');
     });
@@ -500,7 +517,7 @@ describe('Reflect', function () {
     });
   });
 
-  describeIfES5('Reflect.preventExtensions()', function () {
+  describe('Reflect.preventExtensions()', function () {
     it('is a function', function () {
       expect(typeof Reflect.preventExtensions).to.equal('function');
     });