-
Notifications
You must be signed in to change notification settings - Fork 67
avoid constructor.name pattern #55
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
Conversation
Signed-off-by: Yahya <[email protected]>
Thanks a lot @ya7ya. I just changed the commit message to match the JS Code Guidelines. |
@@ -415,7 +415,7 @@ Multiaddr.protocols = protocols | |||
*/ | |||
Multiaddr.isMultiaddr = function isMultiaddr (addr) { | |||
if (addr.constructor && addr.constructor.name) { | |||
return addr.constructor.name === 'Multiaddr' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, this will fail terribly when we have multiple versions of Multiaddr flying around. Which happens often given the myriad of modules we have that import Multiaddr and also the user apps.
@@ -415,7 +415,7 @@ Multiaddr.protocols = protocols | |||
*/ | |||
Multiaddr.isMultiaddr = function isMultiaddr (addr) { | |||
if (addr.constructor && addr.constructor.name) { | |||
return addr.constructor.name === 'Multiaddr' | |||
return (addr instanceof Multiaddr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diasdavid Sorry I thought the revert was already merged, but it was only approved. |
trying to fix ipfs/js-ipfs#1131 properly.
cc @diasdavid
Signed-off-by: Yahya [email protected]