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

deepEqual behaving unexpectedly with empty object #127

Closed
bruderstein opened this issue Feb 22, 2015 · 0 comments
Closed

deepEqual behaving unexpectedly with empty object #127

bruderstein opened this issue Feb 22, 2015 · 0 comments
Assignees
Labels
bug
Milestone

Comments

@bruderstein
Copy link

@bruderstein bruderstein commented Feb 22, 2015

This issue originally raised as hapijs/code#15, and it was suggested to raise it here.

Due to Hoek's deepEqual funtion only checking existing properties in the "left" object, thereby allowing the right object to be a superset of the left, means that an empty object is always deeply equal to any object.

e.g.
If you have the following

var auth = getCurrentAuthentication();
if (Hoek.deepEqual(auth, { authenticated: true, roles: ['admin'] })) {
   doImportantStuff();
}

If getCurrentAuthentication() returns an empty object, this will pass, which is probably unexpected.

You get a different (and arguably better, in this case) result reversing the arguments:

if (Hoek.deepEqual({ authenticated: true, roles: ['admin'] }, auth)) {
 ...
}

For Hoek, I can see the use case, where you want to allow the second object to be a superset of the first. I had initially anticipated that the fix should be just to code, and leave Hoek as it is (possibly with an addition to the documentation detailing the feature), but was directed here.

Happy to write a patch, if there's consensus on what deepEqual should do.

@Marsup Marsup closed this in 7ffc68a Feb 26, 2015
nlf added a commit that referenced this issue Feb 26, 2015
Fix deepEqual on empty objects. Fixes #127.
@hueniverse hueniverse added the bug label Mar 5, 2015
@hueniverse hueniverse added this to the 2.11.1 milestone Mar 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.