Removing callbacks on Object.keys and Object.values

Former-commit-id: a16d4cad89bb7fa4a095e6ae0ebe90c3d408db19 [formerly 078d21323f34d0b6878f53faa154745dc5e69666]
Former-commit-id: 9fcef62223446579a2241ff2c8770c418bcf7671

Author: andrewplummer

lib/object.js

@@ -9,9 +9,6 @@
  *
  ***/
 
-// Flag allowing Object.keys to be enhanced
-var OBJECT_ENHANCEMENTS_FLAG = 'enhanceObject';
-
 // Matches bracket-style query strings like user[name]
 var DEEP_QUERY_STRING_REG = /^(.+?)(\[.*\])$/;
 
@@ -25,9 +22,6 @@ var getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
 // Internal reference to check if an object can be serialized.
 var internalToString = Object.prototype.toString;
 
-// Reference to native Object.keys.
-var nativeObjectKeys = Object.keys;
-
 // Query Strings | Creating
 
 function toQueryStringWithOptions(obj, opts) {
@@ -353,32 +347,17 @@ function clone(source, deep) {
 // Keys/Values
 
 function objectSize(obj) {
-  return keysWithObjectCoercion(obj).length;
+  return getKeysWithObjectCoercion(obj).length;
 }
 
-function keysWithObjectCoercion(obj) {
+function getKeysWithObjectCoercion(obj) {
   return getKeys(coercePrimitiveToObject(obj));
 }
 
-function getKeysWithCallback(obj, fn) {
-  if (isFunction(fn)) {
-    var keys = [];
-    forEachProperty(obj, function(key) {
-      fn.call(obj, key, obj);
-      keys.push(key);
-    });
-    return keys;
-  }
-  return nativeObjectKeys(obj);
-}
-
-function getValuesWithCallback(obj, fn) {
+function getValues(obj, fn) {
   var values = [];
   forEachProperty(obj, function(key, val) {
     values.push(val);
-    if (isFunction(fn)) {
-      fn.call(obj, val, obj);
-    }
   });
   return values;
 }
@@ -496,34 +475,6 @@ function buildClassCheckMethods() {
   });
 }
 
-defineInstanceAndStatic(sugarObject, {
-
-  /***
-   * @method keys(<obj>, [fn])
-   * @returns Array
-   * @polyfill ES5
-   * @short Returns an array containing the keys in <obj>.
-   * @extra Sugar enhances this method to allow an optional function that is
-   *        called for each key. Note that the order of returned keys is not
-   *        guaranteed.
-   *
-   * @callback fn
-   *
-   *   key  The key of the current iteration.
-   *   obj  A reference to the object.
-   *
-   * @example
-   *
-   *   Object.keys({ broken: 'wear' }) -> ['broken']
-   *   Object.keys({ broken: 'wear' }, function(key) {
-   *     // Called once for each key.
-   *   });
-   *
-   ***/
-  'keys': fixArgumentLength(getKeysWithCallback)
-
-}, [ENHANCEMENTS_FLAG, OBJECT_ENHANCEMENTS_FLAG]);
-
 defineStatic(sugarObject, {
 
   /***
@@ -573,10 +524,10 @@ defineStatic(sugarObject, {
 defineInstanceAndStatic(sugarObject, {
 
   /***
-   * @method has(<obj>, <key>, [allowPrototype] = false)
+   * @method has(<obj>, <key>, [inherited] = false)
    * @returns Boolean
    * @short Checks if <obj> has property <key>.
-   * @extra Supports `deep properties`. If [allowPrototype] is `true`,
+   * @extra Supports `deep properties`. If [inherited] is `true`,
    *        properties defined in the prototype chain will also return `true`.
    *        The default of `false` for this argument makes this method suited
    *        to working with objects as data stores by default.
@@ -594,10 +545,10 @@ defineInstanceAndStatic(sugarObject, {
   },
 
   /***
-   * @method get(<obj>, <key>, [allowPrototype] = false)
+   * @method get(<obj>, <key>, [inherited] = false)
    * @returns Mixed
    * @short Gets a property of <obj>.
-   * @extra Supports `deep properties`. If [allowPrototype] is `true`,
+   * @extra Supports `deep properties`. If [inherited] is `true`,
    *        properties defined in the prototype chain will also be returned.
    *        The default of `false` for this argument makes this method suited
    *        to working with objects as data stores by default.
@@ -802,7 +753,7 @@ defineInstanceAndStatic(sugarObject, {
    * @method add(<obj1>, <obj2>, [options])
    * @returns Object
    * @short Merges properties in <obj1> and <obj2> and returns a new object.
-   * @extra See `merge` for options.
+   * @extra This method will not modify either object. See `merge` for options.
    *
    * @example
    *
@@ -855,7 +806,7 @@ defineInstanceAndStatic(sugarObject, {
    * @returns Merged object
    * @short Merges properties from one or multiple <sources> into <target> while
    *        preserving <target>'s properties.
-   * @extra See `merge` for options.
+   * @extra This method modifies <target>! See `merge` for options.
    *
    * @example
    *
@@ -884,27 +835,19 @@ defineInstanceAndStatic(sugarObject, {
   },
 
   /***
-   * @method values(<obj>, [fn])
+   * @method values(<obj>)
    * @returns Array
-   * @short Returns an array containing the values in <obj>. Optionally calls
-   *        [fn] for each value.
-   * @extra Values are in no particular order.
-   *
-   * @callback fn
-   *
-   *   val  The value of the current iteration.
-   *   obj  A reference to the object.
+   * @short Returns an array containing the values in <obj>.
+   * @extra Values are in no particular order. Does not include inherited or
+   *        non-enumerable properties.
    *
    * @example
    *
-   *   Object.values({ broken: 'wear' }) -> ['wear']
-   *   Object.values(usersByName, function(user) {
-   *     // Called once for each user.
-   *   });
+   *   Object.values({a:'a',b:'b'}) -> ['a','b']
    *
    ***/
-  'values': function(obj, fn) {
-    return getValuesWithCallback(obj, fn);
+  'values': function(obj) {
+    return getValues(obj);
   },
 
   /***
@@ -975,9 +918,9 @@ defineInstanceAndStatic(sugarObject, {
   /***
    * @method isObject(<obj>)
    * @returns Boolean
-   * @short Returns true if <obj> is a plain object.
-   * @extra Does not include instances of classes or "host" objects, such as
-   *        Elements, Events, etc.
+   * @short Returns true if <obj> is a "plain" object.
+   * @extra Plain objects do not include instances of classes or "host" objects,
+   *        such as Elements, Events, etc.
    *
    * @example
    *
@@ -992,7 +935,7 @@ defineInstanceAndStatic(sugarObject, {
    * @method remove(<obj>, <search>)
    * @returns Object
    * @short Deletes all properties in <obj> matching <search>.
-   * @extra This method implements `enhanced matching`.
+   * @extra This method will modify <obj>!. Implements `enhanced matching`.
    *
    * @callback search
    *
@@ -1014,8 +957,8 @@ defineInstanceAndStatic(sugarObject, {
    * @method exclude(<obj>, <search>)
    * @returns Object
    * @short Returns a new object with all properties matching <search> removed.
-   * @extra This is a non-destructive version of `remove`. This method
-   *        implements `enhanced matching`.
+   * @extra This is a non-destructive version of `remove` and will not modify
+   *        <obj>. Implements `enhanced matching`.
    *
    * @callback search
    *

test/tests/object.js

@@ -770,25 +770,6 @@ namespace('Object', function () {
 
   });
 
-  method('keys', function() {
-
-    var called = false;
-    var fn = function(key, o) {
-      equal(key, 'foo', 'First argument should be key');
-      equal(o, obj, 'Second argument should be the object');
-      called = true;
-    }
-
-    var obj = {foo:'bar'};
-    run(obj, 'keys', [fn]);
-    equal(called, true, 'Callback should have been called');
-
-    // Issue #525
-    var result = [{foo:'foo'},{bar:'bar'}].map(Sugar.Object.keys);
-    equal(result, [['foo'],['bar']], 'non-function argument should not be called');
-
-  });
-
   method('values', function() {
 
     test({foo:'bar'}, ['bar'], 'Values should be received');
@@ -799,9 +780,6 @@ namespace('Object', function () {
       equal(o, obj, 'Second argument should be the object');
       called = true;
     }
-    var obj = {foo:'bar'};
-    var result = run(obj, 'values', [fn]);
-    equal(called, true, 'Callback should have been called');
 
     // Issue #525
     var result = [{foo:'foo'},{bar:'bar'}].map(Sugar.Object.values);