refactor(commons): use named exports

- export every function directly
- improve 'some' & 'every' for early exit
- use 'Object.values' for 'values' instead of 'Object.keys.map' (2x perf)
- isObject: first check for null
- named imports from @feathersjs/commons in every package
- deprecate 'keys', use `Object.keys`
- deprecate 'values', use `Object.values`
- deprecate 'extend', use `Object.assign`
- use type guard for `isPromise`, `isObject` & `isObjectOrArray`
- add jsdoc comments

packages/adapter-commons/src/index.ts

@@ -1,4 +1,4 @@
-import { _ } from '@feathersjs/commons'
+import { pick } from '@feathersjs/commons'
 import { Params } from '@feathersjs/feathers'
 
 export * from './declarations'
@@ -17,7 +17,7 @@ export function select(params: Params, ...otherFields: string[]) {
   }
 
   const resultFields = queryFields.concat(otherFields)
-  const convert = (result: any) => _.pick(result, ...resultFields)
+  const convert = (result: any) => pick(result, ...resultFields)
 
   return (result: any) => {
     if (Array.isArray(result)) {

packages/adapter-commons/src/query.ts

@@ -1,11 +1,11 @@
-import { _ } from '@feathersjs/commons'
+import { isObject } from '@feathersjs/commons'
 import { BadRequest } from '@feathersjs/errors'
 import { Query } from '@feathersjs/feathers'
 import { FilterQueryOptions, FilterSettings, PaginationParams } from './declarations'
 
 const parse = (value: any) => (typeof value !== 'undefined' ? parseInt(value, 10) : value)
 
-const isPlainObject = (value: any) => _.isObject(value) && value.constructor === {}.constructor
+const isPlainObject = (value: any) => isObject(value) && value.constructor === {}.constructor
 
 const validateQueryProperty = (query: any, operators: string[] = []): Query => {
   if (!isPlainObject(query)) {

packages/authentication-oauth/src/strategy.ts

@@ -6,7 +6,7 @@ import {
 } from '@feathersjs/authentication'
 import { Params } from '@feathersjs/feathers'
 import { NotAuthenticated } from '@feathersjs/errors'
-import { createDebug, _ } from '@feathersjs/commons'
+import { createDebug, omit } from '@feathersjs/commons'
 import qs from 'qs'
 
 const debug = createDebug('@feathersjs/authentication-oauth/strategy')
@@ -126,7 +126,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
 
     debug('createEntity with data', data)
 
-    return this.entityService.create(data, _.omit(params, 'query'))
+    return this.entityService.create(data, omit(params, 'query'))
   }
 
   async updateEntity(entity: any, profile: OAuthProfile, params: Params) {
@@ -135,7 +135,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
 
     debug(`updateEntity with id ${id} and data`, data)
 
-    return this.entityService.patch(id, data, _.omit(params, 'query'))
+    return this.entityService.patch(id, data, omit(params, 'query'))
   }
 
   async getEntity(result: any, params: Params) {
@@ -151,7 +151,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
     }
 
     return entityService.get(result[entityId], {
-      ..._.omit(params, 'query'),
+      ...omit(params, 'query'),
       [entity]: result
     })
   }

packages/commons/src/index.ts

@@ -5,98 +5,182 @@ export function stripSlashes(name: string) {
 
 export type KeyValueCallback<T> = (value: any, key: string) => T
 
-// A set of lodash-y utility functions that use ES6
-export const _ = {
-  each(obj: any, callback: KeyValueCallback<void>) {
-    if (obj && typeof obj.forEach === 'function') {
-      obj.forEach(callback)
-    } else if (_.isObject(obj)) {
-      Object.keys(obj).forEach((key) => callback(obj[key], key))
+/**
+ * If the object is an array, it will iterate over every element. 
+ * Otherwise, it will iterate over every key in the object.
+ */
+export function each(obj: Record<string, any> | any[], callback: KeyValueCallback<void>) {
+  if (Array.isArray(obj)) {
+    obj.forEach(callback as any)
+  } else if (isObject(obj)) {
+    Object.keys(obj).forEach((key) => callback(obj[key], key))
+  }
+}
+
+/**
+ * Check if some values in the object pass the test implemented by the provided function
+ * 
+ * returns true if some values pass the test, otherwise false
+ * 
+ * returns false if the object is empty
+ */
+export function some(value: Record<string, any>, callback: KeyValueCallback<boolean>) {
+  for (const key in value) {
+    if (callback(value[key], key)) {
+      return true
     }
-  },
-
-  some(value: any, callback: KeyValueCallback<boolean>) {
-    return Object.keys(value)
-      .map((key) => [value[key], key])
-      .some(([val, key]) => callback(val, key))
-  },
-
-  every(value: any, callback: KeyValueCallback<boolean>) {
-    return Object.keys(value)
-      .map((key) => [value[key], key])
-      .every(([val, key]) => callback(val, key))
-  },
-
-  keys(obj: any) {
-    return Object.keys(obj)
-  },
-
-  values(obj: any) {
-    return _.keys(obj).map((key) => obj[key])
-  },
-
-  isMatch(obj: any, item: any) {
-    return _.keys(item).every((key) => obj[key] === item[key])
-  },
-
-  isEmpty(obj: any) {
-    return _.keys(obj).length === 0
-  },
-
-  isObject(item: any) {
-    return typeof item === 'object' && !Array.isArray(item) && item !== null
-  },
-
-  isObjectOrArray(value: any) {
-    return typeof value === 'object' && value !== null
-  },
-
-  extend(first: any, ...rest: any[]) {
-    return Object.assign(first, ...rest)
-  },
-
-  omit(obj: any, ...keys: string[]) {
-    const result = _.extend({}, obj)
-    keys.forEach((key) => delete result[key])
-    return result
-  },
+  }
 
-  pick(source: any, ...keys: string[]) {
-    return keys.reduce((result: { [key: string]: any }, key) => {
-      if (source[key] !== undefined) {
-        result[key] = source[key]
-      }
+  return false
+}
 
-      return result
-    }, {})
-  },
-
-  // Recursively merge the source object into the target object
-  merge(target: any, source: any) {
-    if (_.isObject(target) && _.isObject(source)) {
-      Object.keys(source).forEach((key) => {
-        if (_.isObject(source[key])) {
-          if (!target[key]) {
-            Object.assign(target, { [key]: {} })
-          }
-
-          _.merge(target[key], source[key])
-        } else {
-          Object.assign(target, { [key]: source[key] })
-        }
-      })
+/**
+ * Check if all values in the object pass the test implemented by the provided function
+ * 
+ * returns true if all values pass the test, otherwise false
+ * 
+ * returns true if the object is empty
+ */
+export function every(value: any, callback: KeyValueCallback<boolean>) {
+  for (const key in value) {
+    if (!callback(value[key], key)) {
+      return false
     }
+  }
+
+  return true
+}
+
+/**
+ * @deprecated use `Object.keys` instead
+ */
+export function keys(obj: any) {
+  return Object.keys(obj)
+}
+
+/**
+ * @deprecated use `Object.values` instead
+ */
+export function values(obj: any) {
+  return Object.values(obj)
+}
+
+/**
+ * Check if values in the source object are equal to the target object
+ * 
+ * Does a shallow comparison
+ */
+export function isMatch(target: any, source: any) {
+  return Object.keys(source).every((key) => target[key] === source[key])
+}
+
+/**
+ * Check if the object is empty
+ */
+export function isEmpty(obj: any) {
+  return Object.keys(obj).length === 0
+}
+
+/**
+ * Check if the item is an object and not an array
+ */
+export function isObject(item: any): item is Record<string, any> {
+  return item !== null && typeof item === 'object' && !Array.isArray(item)
+}
+
+/**
+ * Check if the value is an object or an array
+ */
+export function isObjectOrArray(value: any): value is Record<string, any> | any[] {
+  return value !== null && typeof value === 'object'
+}
+
+/**
+ * @deprecated use `Object.assign` instead.
+ */
+export function extend(first: any, ...rest: any[]) {
+  return Object.assign(first, ...rest)
+}
+
+/**
+ * Return a shallow copy of the object with the keys removed
+ */
+export function omit(obj: any, ...keys: string[]) {
+  const result = { ...obj }
+  keys.forEach((key) => delete result[key])
+  return result
+}
+
+/**
+ * Return a shallow copy of the object with only the keys provided
+ */
+export function pick(source: any, ...keys: string[]) {
+  return keys.reduce((result: { [key: string]: any }, key) => {
+    if (source[key] !== undefined) {
+      result[key] = source[key]
+    }
+
+    return result
+  }, {})
+}
+
+/**
+ * Recursively merge the source object into the target object
+ */
+export function merge(target: any, source: any) {
+  // If either the source or target are not objects, there is nothing to do
+  if (!isObject(target) || !isObject(source)) {
     return target
   }
+
+  Object.keys(source).forEach((key) => {
+    if (isObject(source[key])) {
+      if (!target[key]) {
+        Object.assign(target, { [key]: {} })
+      }
+
+      merge(target[key], source[key])
+    } else {
+      Object.assign(target, { [key]: source[key] })
+    }
+  })
+
+  return target
 }
 
-// Duck-checks if an object looks like a promise
-export function isPromise(result: any) {
-  return _.isObject(result) && typeof result.then === 'function'
+/**
+ * Duck-checks if an object looks like a promise
+ */
+export function isPromise(result: any): result is Promise<any> {
+  return isObject(result) && typeof result.then === 'function'
 }
 
 export function createSymbol(name: string) {
   return typeof Symbol !== 'undefined' ? Symbol.for(name) : name
 }
 
+/**
+ * A set of lodash-y utility functions that use ES6
+ *
+ * @deprecated Don't use `import { _ } from '@feathersjs/commons'`. You're importing a bunch of functions. You probably only need a few. 
+ * Import them directly instead. For example: `import { merge } from '@feathersjs/commons'`.
+ * 
+ * If you really want to import all functions or do not care about cherry picking, you can use `import * as _ from '@feathersjs/commons'`.
+ */
+export const _ = {
+  each,
+  some,
+  every,
+  keys,
+  values,
+  isMatch,
+  isEmpty,
+  isObject,
+  isObjectOrArray,
+  extend,
+  omit,
+  pick,
+  merge
+}
+
 export * from './debug'

packages/commons/test/module.test.ts

@@ -1,5 +1,6 @@
 import { strict as assert } from 'assert'
 import { _ } from '../src'
+import * as commons from '../src'
 
 describe('module', () => {
   it('is commonjs compatible', () => {
@@ -9,6 +10,18 @@ describe('module', () => {
     assert.equal(typeof commons, 'object')
     assert.equal(typeof commons.stripSlashes, 'function')
     assert.equal(typeof commons._, 'object')
+    assert.equal(typeof commons.each, 'function')
+    assert.equal(typeof commons.some, 'function')
+    assert.equal(typeof commons.every, 'function')
+    assert.equal(typeof commons.keys, 'function')
+    assert.equal(typeof commons.values, 'function')
+    assert.equal(typeof commons.isMatch, 'function')
+    assert.equal(typeof commons.isEmpty, 'function')
+    assert.equal(typeof commons.isObject, 'function')
+    assert.equal(typeof commons.extend, 'function')
+    assert.equal(typeof commons.omit, 'function')
+    assert.equal(typeof commons.pick, 'function')
+    assert.equal(typeof commons.merge, 'function')
   })
 
   it('exposes lodash methods under _', () => {
@@ -25,4 +38,19 @@ describe('module', () => {
     assert.equal(typeof _.pick, 'function')
     assert.equal(typeof _.merge, 'function')
   })
+
+  it('exposes separate methods under _', () => {
+    assert.equal(typeof commons.each, 'function')
+    assert.equal(typeof commons.some, 'function')
+    assert.equal(typeof commons.every, 'function')
+    assert.equal(typeof commons.keys, 'function')
+    assert.equal(typeof commons.values, 'function')
+    assert.equal(typeof commons.isMatch, 'function')
+    assert.equal(typeof commons.isEmpty, 'function')
+    assert.equal(typeof commons.isObject, 'function')
+    assert.equal(typeof commons.extend, 'function')
+    assert.equal(typeof commons.omit, 'function')
+    assert.equal(typeof commons.pick, 'function')
+    assert.equal(typeof commons.merge, 'function')
+  })
 })