flitbit / diff

Javascript utility for calculating deep difference, capturing changes, and applying changes across objects; for nodejs and the browser.
MIT License
3k stars 214 forks source link

Cannot read property 'hasOwnProperty' of undefined in 0.3.7 #102

Closed ParaSwarm closed 6 years ago

ParaSwarm commented 7 years ago

When deep diff'ing a complex object with nested complex objects, I am now getting this error after today's release of 0.3.7:

error_handler.js:60 TypeError: Cannot read property 'hasOwnProperty' of undefined at deepDiff (http://localhost:8080/bundle.js?c041ec458b1252450f55:132079:79) at http://localhost:8080/bundle.js?c041ec458b1252450f55:132112:13 at Array.forEach (native) at deepDiff (http://localhost:8080/bundle.js?c041ec458b1252450f55:132109:15) at Object.accumulateDiff [as default] (http://localhost:8080/bundle.js?c041ec458b1252450f55:132136:3) at SafeSubscriber._next (http://localhost:8080/bundle.js?c041ec458b1252450f55:112959:52) at SafeSubscriber.__tryOrUnsub (http://localhost:8080/bundle.js?c041ec458b1252450f55:5080:16) at SafeSubscriber.next (http://localhost:8080/bundle.js?c041ec458b1252450f55:5029:22) at Subscriber._next (http://localhost:8080/bundle.js?c041ec458b1252450f55:4971:26) at Subscriber.next (http://localhost:8080/bundle.js?c041ec458b1252450f55:4935:18)

This was working fine in 0.3.6.

ParaSwarm commented 7 years ago

This issue occurs when there is an undefined array of complex objects in my parent object.

Removing the array or populating it causes the issue to not occur.

marjan-georgiev commented 7 years ago

I can confirm this. My app started throwing this error after 0.3.7 was released.

marjan-georgiev commented 7 years ago

This is the line throwing the error https://github.com/flitbit/diff/blob/master/index.js#L154

stack[stack.length - 1].lhs can be undefined.

Kamilius commented 7 years ago

Same here. Unit tests of components using deepdiff started crashing after 0.3.7 update. I'm passing two arrays of plain objects like following:

diff([{
  key: 'Option 1',
  value: 1,
}], [{
  key: 'Option 2',
  value: 2,
  randomProp: 'string',
}]),

And this is throwing exact same error, as mentioned above. Fails on line 147 of deep-diff's index.es.js:

var ldefined = ltype !== 'undefined' || stack && stack[stack.length - 1].lhs.hasOwnProperty(key);

Error msg: "Uncaught TypeError: Cannot read property 'hasOwnProperty' of undefined"

Stack item by a given index doesn't have lhs/rhs properties as figured out during debug.

flitbit commented 7 years ago

Please review with vsn 0.3.8. I expect this issue is fixed. Even better, if someone can contribute a unit test to catch future regression, I'd appreciate it!

aleexkj commented 7 years ago

On version 0.3.8, got: Uncaught TypeError: Cannot read property 'rhs' of undefined when diff 2 and undefined just as @Kamilius pointed out. Pleas some fix, it's a dependency for waterline and it causes a mess. In last version, the previous if instead of the vars ldefined, rdefined worked fine.. but idk the repercussions on the new update.

jrthib commented 7 years ago

I'm getting Uncaught TypeError: Cannot read property 'lhs' of undefined on 0.3.8

ryanlaws commented 7 years ago

I'm getting the same behavior with 0.3.8 and it's exhibited whether lhs or rhs is undefined.

imevro commented 7 years ago

@flitbit ?

Underdoge commented 7 years ago

Same here, getting the following with version 0.3.8:

index.bundle.js:1174 Uncaught TypeError: Cannot read property 'lhs' of undefined at deepDiff (index.bundle.js:1174) at Function.accumulateDiff [as diff] (index.bundle.js:1231) at GridPaint.exports.compare [as compareChanges] (index.bundle.js:2535) at GridPaint.module.exports [as clear] (index.bundle.js:2237) at new GridPaint (index.bundle.js:2116) at newPainter (index.bundle.js:5586) at module.exports (index.bundle.js:5616) at Object.register [as use] (index.bundle.js:926) at Object.61.../node_modules/bulma/css/bulma.css (index.bundle.js:5014) at s (index.bundle.js:1)`

cpcallen commented 7 years ago

Here is a minimal reproduction of this bug (in 0.3.8):

diff(Object.create(null), {bar: undefined});

The problem is that is is not safe to blindly call the .hasOwnProperty on an object, because objects created using Object.create(null) do not have that property key. It would be better to use Object.getOwnPropertyDescriptor().

andyburke commented 7 years ago

This seems like a pretty bad bug to go unaddressed for this long. Is there an ETA on getting #104 merged and a fixed version published?

cpcallen commented 7 years ago

Unfortunately #104 does not fully fix the problem. I think #112 is closer to correct, but more tests are welcome!

Kamilius commented 7 years ago

Any updates on this one?

awdDev786 commented 6 years ago

npm install deep-diff@0.3.2 removes the issue of

cannot read property 'rhs' of undefined.

Please fix the issue as it is breaking waterline's code on model.save() query.

flitbit commented 6 years ago

The fix is in version 1.0.0-pre.1 and above...

Has not been published to a full version yet... I'm still optimizing and unrolling the recursion. You can install this version using:

npm install -S deep-diff@next