Skip to content
This repository has been archived by the owner on Apr 20, 2018. It is now read-only.

internal isEqual is broken #119

Closed
acthp opened this issue Mar 11, 2014 · 3 comments
Closed

internal isEqual is broken #119

acthp opened this issue Mar 11, 2014 · 3 comments
Assignees

Comments

@acthp
Copy link

acthp commented Mar 11, 2014

From the debugger:

isEqual({samples:['a'],order:[1,2]}, {samples:['a'],order:[3,4]})
true

contrasting with underscore:

_.isEqual({samples:['a'],order:[1,2]}, {samples:['a'],order:[3,4]})
false
@acthp
Copy link
Author

acthp commented Mar 11, 2014

I suspect the problem is line 223 of rx.js. It looks like this used to do a forEach on the property names, using a callback to return the recursive equality results. Only now it's a for..in, and the return on line 223 exits deepEquals after checking only the first property.

Also, line 232 below this also looks broken in the same way. It looks very much like a forEach -> for..in conversion. unfortunately, I can't find a commit where this happened.

@mattpodwysocki mattpodwysocki self-assigned this Mar 13, 2014
@mattpodwysocki
Copy link
Member

Will investigate

mattpodwysocki added a commit that referenced this issue Mar 14, 2014
@mattpodwysocki
Copy link
Member

Fixed as per your request. The following tests will now pass/fail:

isEqual({samples:['a'],order:[1,2]}, {samples:['a'],order:[3,4]})
// => false
isEqual({samples:['a'],order:[1,2]}, {samples:['a'],order:[1,2]})
// => true

bouzuya pushed a commit to bouzuya/RxJS that referenced this issue Mar 23, 2016
feat(index): add index module which requires commonjs build
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants