lib: change abstract equal to strict equal #22974
Conversation
nodejs-github-bot
added
the
console
| @@ -349,7 +349,7 @@ Console.prototype.table = function(tabularData, properties) { | |||
| if (properties !== undefined && !ArrayIsArray(properties)) | |||
| throw new ERR_INVALID_ARG_TYPE('properties', 'Array', properties); | |||
|
|
|||
| if (tabularData == null || typeof tabularData !== 'object') | |||
| if (tabularData === null || typeof tabularData !== 'object') | |||
There was a problem hiding this comment.
It's not obvious, but this significantly changes the semantics of the check in a non-backwards compatible way.
null == null; // true
undefined == null; // true
null === null; // true
undefined === null; // falseThe current check makes sure that tabularData is also not undefined.
There was a problem hiding this comment.
But the later one can check it, typeof undefined === 'undefined' so typeof tabularData !== 'object' is true.
There was a problem hiding this comment.
This is 100 % subjective, but maybe this would be a bit more Obviously Correct™ if the conditions were switched?
There was a problem hiding this comment.
Just not seeing the point of the change then if it simply shifts the undefined case a bit to the right.
There was a problem hiding this comment.
Just change to strict equal to unify the overall code style.
If there is any problem, feel free to close it.😜
There was a problem hiding this comment.
Nah, I'm not overly happy with it but I won't block it :-)
addaleax
added
the
author ready
|
Resumed build: https://ci.nodejs.org/job/node-test-pull-request/17424/ |
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
|
Thanks all of you. |
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Just change the rare
==to===, nothing else be affected.Unify the style, may looks more comfortable 😄.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes