QubitProducts / slapdash

A lightweight JavaScript utility belt with native method override protection
http://npmjs.org/packages/slapdash
MIT License
3 stars 1 forks source link

Todo #20

Closed alanclarke closed 8 years ago

alanclarke commented 8 years ago

this is looking awesome, couple of things left todo

build

research

jimmed commented 8 years ago

Research Outcomes

Using a patched version of underscore in smartserve with kn8.lt and every simple mode component available in place, the following call counts* can be found:

N.B. Aliased methods have been joined together here (i.e. all and every)

{
  "bind": 10534,
  "isObject": 5594,
  "isArray": 1520,
  "isFunction": 1072,
  "isEqual": 1022,
  "reduce": 1012,
  "isDate": 1006,
  "isRegExp": 1006,
  "keys": 994,
  "has": 831,
  "each": 795,
  "first": 312,
  "isString": 263,
  "map": 258,
  "isEmpty": 249,
  "isFinite": 177,
  "isArguments": 134,
  "pluck": 122,
  "find": 222,
  "filter": 118,
  "extend": 71,
  "result": 56,
  "identity": 41,
  "every": 41,
  "uniqueId": 32,
  "contains": 29,
  "size": 12,
  "clone": 9,
  "pick": 8,
  "isUndefined": 5,
  "random": 5,
  "chain": 2,
  "compact": 2,
  "flatten": 2,
  "memoize": 2,
  "throttle": 2,
  "bindAll": 1,
  "compose": 1,
  "debounce": 1,
  "defaults": 1,
  "difference": 1,
  "groupBy": 1,
  "invoke": 1,
  "isBoolean": 1,
  "isNumber": 1,
  "range": 1,
  "shuffle": 1,
  "sortBy": 1,
}

For reference, here's the patch (added at the very bottom of underscore, just inside its IIFE):

  window.slappinItDashwards = {}
  _.functions(_).forEach(function (fnName) {
    window.slappinItDashwards[fnName] = 0
    var orig = _[fnName]
    _[fnName] = function () {
      window.slappinItDashwards[fnName]++
      return orig.apply(this, arguments)
    }
  })
alanclarke commented 8 years ago

this is super interesting and raises some points for discussion:

jimmed commented 8 years ago

Calls to _.bind

The main culprit is VariationManager - for each call to VariationManager(), bind is called for each method of vm

alanclarke commented 8 years ago

https://github.com/qubitdigital/experience-engine/pull/32

jimmed commented 8 years ago

there are 9k calls to isArray, isObject, isDate and isRegexp combined. These are useful basically where there are ambiguous apis. We shouldn't have any ambiguous apis, and should be able to assume that things are called with the correct arguments

  • Definitely agreed on this. The only caveat here is that if we do need to consume an ambiguous API, Array.isArray is not available as a fallback in IE8.

what do we think about isFunction vs typeof val === 'function'? 1k calls

  • For types other than arrays, a simple typeof check should suffice. Lodash's isX methods do a toString check which I highly doubt we'll need.

is findWhere easily avoided? perhaps we should include this

  • It's an alias of find, which we already implement. Have merged the stats together above.

contains has a native equivalent i.e. includes and perhaps we should provide safe access to it

  • It's basically just indexOf with an extra optional parameter. In fact, all 29 calls actually an internal dependency of _.difference - i.e. we never call contains directly.
jimmed commented 8 years ago

On isArray

As discussed above, we need to decide whether to include a wrapper/featurefill for Array.isArray, as IE8 doesn't have it.

By far the most common (and only) usage pattern is something like this:

x = isArray(x) ? x : [x]

...in other words, overly flexible APIs.

I would argue that in these circumstances, we should do everything we can to ensure these methods accept only an array. Ergo, bye bye isArray!

davidpadbury commented 8 years ago

I've just run a quick search over the modules which @qubitdigital/segments use client side to look at our current underscore usage. (used: ag -o --nocolor --noheading --nobreak --nofilename '_\.(\w+)' ssg-visitor-engine visitor-engine visitor-engine-segments | sort -n | uniq -c | sort -nr).

Results are:

  20 _.each
  13 _.reduce
  11 _.map
  10 _.keys
  10 _.extend
   9 _.partial
   9 _.isArray
   6 _.some
   6 _.difference
   5 _.isUndefined
   5 _.isString
   5 _.isFunction
   4 _.union
   4 _.invert
   4 _.defaults
   4 _.clone
   3 _.every
   3 _.contains
   2 _.last
   2 _.isObject
   2 _.isEmpty
   2 _.filter
   2 _.bind
   2 _.all
   1 _.toArray
   1 _.size
   1 _.pick
   1 _.isNumber
   1 _.isNull
   1 _.isNaN
   1 _.isEqual
   1 _.indexOf
   1 _.flatten
   1 _.first
   1 _.findWhere
   1 _.find
   1 _.compose
   1 _.chain
   1 _.any

cc: @qubitdigital/segments for anyone interested.

alanclarke commented 8 years ago

isObject is not quite trivial to replicate. typeof won't cut it. typeof null === 'object' for example

alanclarke commented 8 years ago

findWhere is not an alias of find btw

jimmed commented 8 years ago

isObject is not quite trivial to replicate

Not quite, but almost. The full implementation looks something like x && indexOf(['function', 'object'], typeof x) > -1.

findWhere is not an alias of find btw

It depends which version of lodash/underscore you ask.

alanclarke commented 8 years ago

lets have _.isMatch method to facilitate converting the 100 odd findWheres