JedWatson / react-tappable

Tappable component for React
http://jedwatson.github.io/react-tappable/
MIT License
863 stars 90 forks source link

Updated extend function to take multiple args #8

Closed nmn closed 9 years ago

nmn commented 9 years ago

Doesn't actually benefit this library, but might be useful in the future.

JedWatson commented 9 years ago

I agree it's nicer but I wonder if there's a performance hit from the added complexity? This is a really high-use component in UIs that have to be super responsive so literally every ms counts.

Not that it's a lot of added complexity, but I don't have any benchmarks handy - would like to test it first.

nmn commented 9 years ago

By all means close this if you want. I just saw that extend method and wanted to make it work for multiple args.

I haven't tested it, but given the code, it should be about the same speed with slightly more RAM usage. There is no way this is faster as javascript doesn't have tail calls yet.

This should be just as fast though:

function extend(target, source) {
  var args = Array.prototype.slice.call(arguments, 2);
  while(true){
    if (!source || Object.prototype.toString.call(source) !== '[object Object]') return target;
    for (var key in source) {
      if (source.hasOwnProperty(key)) {
        target[key] = source[key];
      }
    }
    if(args.length <= 0) {
      return target;  
    }
    source = args[0];
    args = args.slice(1);
  }
}

Some manual tail call optimisation there!

JedWatson commented 9 years ago

@nmn epic.

Going to do some benchmarking and will implement the simplest performant option, leaving this open until then.

Thanks :smile:

nmn commented 9 years ago

This new commit should make things more reliable. The Object.assign method included with React does the same thing as the extend function and has been tested for speed and correctness. Also, this is an ES6 feature, so it is possible to remove it in the future.

JedWatson commented 9 years ago

Good idea :)