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

toStrictEqual does not cover sparse arrays #7586

Closed
marcfallows opened this issue Jan 8, 2019 · 6 comments
Closed

toStrictEqual does not cover sparse arrays #7586

marcfallows opened this issue Jan 8, 2019 · 6 comments
Labels

Comments

@marcfallows
Copy link
Contributor

I'm getting what I consider unexpected behaviour for toStrictEqual, treating sparse arrays incorrectly:

expect([undefined, 1]).toStrictEqual([, 1]); // will pass :(

I would expect this assertion to fail.

While in terms of index access it seems these arrays are equivalent (because [,1][0] === undefined), sparse arrays do behave differently:

> [undefined, undefined, undefined, 1].forEach(x => console.log(x))
undefined
undefined
undefined
1

> [, , , 1].forEach(x => console.log(x))
1

This can also be seen as follows:

Object.keys([undefined, undefined, undefined, 1]); // ["0", "1", "2", "3"]
Object.keys([,,, 1]); // ["3"]

And so I don't think it's appropriate to consider these strictly equal.

Happy to contribute a fix, but just want to verify that this would be something you agree is a bug.

@jeysal
Copy link
Contributor

jeysal commented Jan 8, 2019

I agree this is a bug, especially since the docs say "Keys with undefined properties are checked. e.g. {a: undefined, b: 2} does not match {b: 2} when using .toStrictEqual." and do not state that it might work differently for arrays.
cc @SimenB @thymikee

@jeysal
Copy link
Contributor

jeysal commented Jan 8, 2019

@emilsjolander (contributed toStrictEqual originally) do you agree?

@emilsjolander
Copy link
Contributor

Agreed, this was an oversight

@jeysal
Copy link
Contributor

jeysal commented Jan 8, 2019

@marcfallows sounds like a nicely isolated bug for a first Jest PR :)

@marcfallows
Copy link
Contributor Author

PR raised: #7591

captain-yossarian pushed a commit to captain-yossarian/jest that referenced this issue Jul 18, 2019
…estjs#7591)

* fix: toStrictEqual considers array sparseness (resolves jestjs#7586)

* changelog

* review feedback

* line in docs
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants