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 and array properties #238

Closed
kanongil opened this issue Feb 24, 2018 · 0 comments · Fixed by #242
Closed

deepEqual and array properties #238

kanongil opened this issue Feb 24, 2018 · 0 comments · Fixed by #242
Assignees
Labels
bug
Milestone

Comments

@kanongil
Copy link
Member

@kanongil kanongil commented Feb 24, 2018

Currently deepEqual() ignores any properties on arrays. Eg:

const a = [];
a.ok = true;
const b = [];

Hoek.deepEqual(a, b);   // result => true

I'm working on a performance optimized deepEqual() refactor, which also fixes #219 & #236, along with other issues.

Unfortunately this issue is quite costly to fix. The iteration is ~100x slower for a 10000 element array. Is it worth it to fix the issue?

Note that the built-in assert.deepEqual(a, b) throws an AssertionError for this. Also worth noting, is that Hoek.clone(a) copies the property to the clone. Given this, I'd say that we need to do a full comparison.

@kanongil kanongil added the bug label Feb 24, 2018
@hueniverse hueniverse added this to the 6.0.0 milestone Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.