Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
YuZhaoping opened this issue Apr 25, 2017 · 1 comment
Closed

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

YuZhaoping opened this issue Apr 25, 2017 · 1 comment
Assignees
Labels

Comments

@YuZhaoping
Copy link

YuZhaoping commented Apr 25, 2017

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
Copy link
Collaborator

ljharb commented Apr 25, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants