-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Re-enable non-strict comparisons on assert module #28780
Comments
@bruno-brant realistically, the legacy assert module can never be removed (or even runtime deprecated) as it would be too disruptive to the ecosystem. There is also nothing fundamentally wrong with the legacy module as long as you understand the difference between strict and loose comparisons. I'd suggest using the legacy module if it best fits your need. |
I would go further and say that using @bruno-brant Is the code base where this came up a publicly available one? I'd be interested to see how this kind of thing comes up in the real world. I'm interested in obstacles people face that keep them using loose equalities. I'm especially interested if this only came up with the simple equality functions or if you had it come up with one of the deep-equal functions too. |
I can actually upload it to a public repo, but the reason I'm using it is actually to run tests. Instead of installing chai or any other assertion library, I'm calling |
Using the legacy assert module is not discouraged. Revoke DEP0089 to avoid user confusion. PR-URL: nodejs#28892 Fixes: nodejs#28780 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Using the legacy assert module is not discouraged. Revoke DEP0089 to avoid user confusion. PR-URL: #28892 Fixes: #28780 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
A js quirk that can bite you is the fact that zeroes can be negative signed. Signs are ignored in this case when comparing numbers by equality, so -0 == 0, and even -0 === 0. However, -0 isn't 0, so
Object.is
returns false when comparing those values.The thing is that the strictEqual function inside the assert module has a very special treatment for zeroes, where it prefers to call
Object.is
:Since the legacy (non-strict) mode is deprecated, one can no longer easily assert that a statement will have a result of 0 without knowing whether or not that statement will result in a negative zero.
So I suggest to either remove the special treatment for negative zeroes (which might be a breaking change) or enable legacy mode.
The current work around is moving from
assert.equal(fn(), 0)
to
assert(fn() === 0)
which results in worse error messages.
The text was updated successfully, but these errors were encountered: