justmoon / node-extend

Simple function to extend objects
MIT License
341 stars 68 forks source link

Should unify the 'target' arg's protection #46

Closed YuZhaoping closed 7 years ago

YuZhaoping commented 7 years ago

While I reviewed the code recently, I've found the 'target' arg's protection is diffent between the 'deep' arg is true or not:

var extend = require('extend');
var result = extend(3.14, {a: 'b'});

this demo works well, but the following demo causes the "TypeError: Cannot create property 'a' on number '3.14'":

var extend = require('extend');
var result = extend(true, 3.14, {a: 'b'});

I see this is because the 'target' arg getting the diffent protecting while the 'deep' arg is true or not. Should it modify the code of index.js file from line 42 to line 49 like this:

if (typeof target === 'boolean') {
  deep = target;
  target = arguments[1];
  // skip the boolean and the target
  i = 2;
}

if ((typeof target !== 'object' && typeof target !== 'function') || target == null) {
  target = {};
}
ljharb commented 7 years ago

node-extend is modeled after $.extend from jQuery.

$.extend(3.14, { a: 'b' }) produces { a: 'b' }, as does $.extend(true, 3.14, { a: 'b' }) - so therefore I think this is a bug.