Hello!

node-extend works slightly different from original jQuery method. For example,

extend({ a: 100}, { b: undefined}) returns { a: 100, b: undefined }

while

$.extend({ a: 100}, { b: undefined}) returns { a: 100 }

I think, jQuery behaviour is more likely in many cases.

There are use-cases in which it makes sense to set a property to undefined and still expect it to be copied. Object.assign() does not filter out such properties, either.

What's the reason for them to be filtered in node-extend?

I was just wondering why this extend() does more then for example this one: https://gist.github.com/coderofsalvation/8239d9362744896f766c

why the isArray stuff?
Is it for browser compatibility reasons?

why do you define hasOwn variable as shorthand to Object.prototype.hasOwnProperty

var hasOwn = Object.prototype.hasOwnProperty;

but use implicity defined global hasOwnProperty?

    var has_own_constructor = hasOwnProperty.call(obj, 'constructor');

is it mistype?

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 = {};
}

When you merge objects that use symbols as keys, they get removed.

I did some research and testing and I believe the issue is here:

The for ... in ignores the symbols, so I tried changing it a little bit:

- for (name in options) {
+ const names = [
+     ...Object.getOwnPropertySymbols(options),
+     ...Object.keys(options)
+ ];
+ for (name of names) {

And now it works.

The spread can be replaced with concat, but getOwnPropertySymbols pretty much leaves IE out and I don't know which are the browser/engines the library should support, that's why I didn't send a PR.

Let me know what you think.
Thanks.

Hi,
Scenario:

var extend=require('extend');
var one = {A:123, B:'ABC', C:{D:'bla', E:[1,2,'3']}};
var two = {A:456, C:{F:789}};
extend(false, res, one, two);
console.log(res); // OUTPUT is: {} (still empty object).

Seems that the issue comes from (index.js line 22):

module.exports = function extend() {
    var options, name, src, copy, copyIsArray, clone,
        target = arguments[0] || {},
            // Since arguments[0] is false then "target" always gets an empty object
            // instead of arguments[1]
            // This solves the issue:
            target = typeof arguments[0] === 'undefined' ? {} : arguments[0],

What do you think?
Thx,
Yuval.

This could be useful on the browser when you don't want to use heavy library like jquery.

I imagine that this is inherited from the jQuery implementation of extend, but I'm experiencing the exact same behavior described here:

http://forum.jquery.com/topic/jquery-extend-modifies-but-not-replaces-array-properties-on-deep-copy

In my opinion, the inclusion of array extending may be more problematic then helpful. Would you consider a deviation on this from what the jQuery folks chose to do?

EDIT: Been giving this some thought, and perhaps consistency is more important in a "port." I can certainly live with the current approach knowing that the overall behavior matches up with jQuery. Still, I'd be interested in your thoughts.

Sometimes, we cannot know target's type, but target must be merged first:

var a = 1, b = 2; extend(true, a, b)

a expect be 2, Could you support this please?

With more and more build tools (vite, nuxt, etc) going towards an esm-first build, it would be nice if a proper esm interface could be provided, i.e. transpile and distribute esm version, including correct export declaration in package.json.

Can support es module? in angular14 should be warnning: depends on 'rfdc'. CommonJS or AMD dependencies can cause optimization bailouts.

How to avoid this warning?

Hi,

The body of the method (in the deep extend branch of the loop) refers to a function extend() but you never declare this. I believe the correct fix is to do the following:

var extend = module.exports = function() { // ...

Thanks, Alex J Burke.

I have a very strange bug on Chrome which throws this error when my development tools aren't open.

I'm using webpack + react + counterpart (which requires this module).

The weird thing is that when I open my development tools, the method isPlainObject is clearly defined, but not when it's closed.

On Safari/FF everything works. It seems like a scope issue when requiring this module via webpack that isPlainObject should be called like _this.isPlainObject and exposing this as _this in the top of the index.js.

Any suggestions?

I just installed npm and started following the instructions at << http://voxeljs.com/#how >>. When I ran npm install I got this message:

npm WARN deprecated [email protected]: Please update to the latest version.

npm update and npm update extend are giving me the same message consistently. I'm guessing this is a problem with some JSON configuration related to extend.

There are probably too many || in there so that the following produces the unexpected:

var
  a = {value: {nodeType: {}}},
  b = extend(true, {}, a)
;

b.value.nodeType === a.value.nodeType;
// true !!!

I think it should rather be

if (toString.call(obj) !== '[object Object]' && (
  !obj || obj.nodeType || obj.setInterval)
) {
  return false;
}

but I am not sure about all those checks and if this could cause regressions (hence the issue instead of a PR)

Cheers

Since it's a port from jQuery, I would assume it's MIT. But can you please specify this?

Extend has a problem with merging arrays.
The elements will be replaced by their indizies
not by uniq values.

Here is a little example:

var extend = require('extend');
var x = extend(
    true,
    {
        x: [700]
    },
    {
        x: [1, 10, 5, 42]
    },
    {
        x: [4, 7]
    }
);

// x = { x: [ 4, 7, 5, 42 ] }

After reading through the documentation and code, it does not seem you can add a callback to the extend function. Is that correct?

{
	"root": true,
	"extends": "@ljharb",
	"rules": {
		"complexity": [2, 20],
		"eqeqeq": [2, "allow-null"],
		"func-name-matching": [2, "always"],
		"max-depth": [1, 4],
		"max-statements": [2, 26],
		"no-magic-numbers": [0],
		"no-restricted-syntax": [2, "BreakStatement", "ContinueStatement", "DebuggerStatement", "LabeledStatement", "WithStatement"],
		"sort-keys": [0],
	},
	"overrides": [
		{
			"files": "test/**",
			"rules": {
				"max-lines-per-function": 0,
			},
		},
	],
}

Trailing commas are not valid in JSON and will cause many parsing mechanisms to report errors.

I saw that you put in a fix for this issue: #43

Could you publish this to npm? We are running into the same problem. Thanks very much.

Hello,

what are the differences between this project and ES6 Object.assign? Are they doing the same thing or not?

[email protected] contains an .editorconfig file that is not in source control. Additionally, there are three config files (.eslintrc, .jscs.json, and .travis.yml) that are not necessary to include in the package. Any of these files are inherently benign; however, the tarball from latest version of the package cannot be consistantly reproduced from source control.

You can reproduce this scan by using "TBV":
npx tbv verify [email protected] --verbose

I recommend adding all root-level dot files to .npmignore. Doing so will allow the package to "verify" and will reduce the unpacked module size by 50%.

See also

• https://www.npmjs.com/package/tbv
• https://hackernoon.com/what-if-we-could-verify-npm-packages-c2a319cff758

Can we rename .eslintrc to .eslintrc.json to prevent .eslintrc from being parsed as YAML?

Cannot read config file: node_modules/extend/.eslintrc
Error: duplicated mapping key at line 45, column 30:
                "beforeLineComment": false,
                                 ^
YAMLException: Cannot read config file: node_modules/extend/.eslintrc
Error: duplicated mapping key at line 45, column 30:
                "beforeLineComment": false,
                                 ^
    at generateError (node_modules/js-yaml/lib/js-yaml/loader.js:162:10)
    at throwError (node_modules/js-yaml/lib/js-yaml/loader.js:168:9)
    at storeMappingPair (node_modules/js-yaml/lib/js-yaml/loader.js:305:7)
    at readFlowCollection (node_modules/js-yaml/lib/js-yaml/loader.js:720:7)
    at composeNode (node_modules/js-yaml/lib/js-yaml/loader.js:1327:11)
    at readFlowCollection (node_modules/js-yaml/lib/js-yaml/loader.js:704:5)
    at composeNode (node_modules/js-yaml/lib/js-yaml/loader.js:1327:11)
    at readFlowCollection (node_modules/js-yaml/lib/js-yaml/loader.js:715:7)
    at composeNode (node_modules/js-yaml/lib/js-yaml/loader.js:1327:11)
    at readFlowCollection (node_modules/js-yaml/lib/js-yaml/loader.js:715:7)

Hey there!

I belong to an open source security research community, and a member (@ranjit-git) has found an issue, but doesn’t know the best way to disclose it.

If not a hassle, might you kindly add a SECURITY.md file with an email, or another contact method? GitHub recommends this best practice to ensure security issues are responsibly disclosed, and it would serve as a simple instruction for security researchers in the future.

Thank you for your consideration, and I look forward to hearing from you!

(cc @huntr-helper)

Not sure how much you care about this, but there's a really tricky bug caused by this module if you try to use Ember.js and IndexedDB in the same application. In short, Ember.js does some modifications to the global String and Array prototypes, so if you try to put() some extended objects into IndexedDB, IndexedDB throws an error saying "an object could not be cloned."

We fixed it in pouchdb/pouchdb#2270 by rolling back to an older version of this function ripped from jQuery 1.9.0. I'm not sure what's been modified in the interim or even what precisely causes the bug. I may be able to open a PR on my end, but it seems tricky enough that I'm not really sure where to start. Thought you might like to know.

Changed the default branch name from master to main.

See: https://www.hanselman.com/blog/EasilyRenameYourGitDefaultBranchFromMasterToMain.aspx

@ljharb - Please update your local working copy:

$ git checkout master
$ git branch -m master main
$ git fetch
$ git branch --unset-upstream
$ git branch -u origin/main
$ git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/main

Any chance of getting an ESM version or some default export shimming introduced? vite dev dislikes the package immensely, throwing an error which resembles:

Uncaught SyntaxError: The requested module '/@fs/private/tmp/jsx-email-starter/node_modules/.pnpm/[email protected]/node_modules/extend/index.js?v=ae42d2d7' does not provide an export named 'default' (at index.js?v=ae42d2d7:350:8)

unified is making use of this package (and is ironically ESM-only) and running vite with anything that wants to use unified is quite impossible without extensive gymnastics.

The .eslintrc file in the npm package is invalid, causing linting checks to fail.

Steps to reproduce:

$ eslint index.js
YAMLException: Cannot read config file: /path/to/node_modules/extend/.eslintrc
Error: duplicated mapping key at line 45, column 30:
"beforeLineComment": false,

Related part of .eslintrc:

  "lines-around-comment": [2, {
        "beforeBlockComment": false,
        "afterBlockComment": false,
        "beforeLineComment": false,
        "beforeLineComment": false,
        "allowBlockStart": true,
        "allowBlockEnd": true
    }],

I assumed I could do var extend = require('extend'); to get the extend function, but I had to look at the source code to confirm this was correct. Maybe be explicit about this by adding the require part to the readme?

I was scanning this project with JSPwn and it seemed to have picked up a possible bug

index.js:64

assignment typo
if($_contains('AssignmentExpression'));

Unintended use of AssignmentExpression in If Statement

Is this intentional?

Prototype pollution vulnerability in function extend() in extend-combine/index.js in [email protected] function extend() lead to Prototype pollution in line 112 and117

It would be incredibly useful to have a Changelog or History.md file to see what changed, in terms of new functionality or breaking changes.

Examples:
https://github.com/visionmedia/mocha/blob/master/History.md
https://github.com/mikeal/request/blob/master/CHANGELOG.md

replace rule "no-empty-label" to "no-labels"