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

isPeerId #57

Merged
merged 3 commits into from
Mar 27, 2017
Merged

isPeerId #57

merged 3 commits into from
Mar 27, 2017

Conversation

daviddias
Copy link
Member

No description provided.

src/index.js Outdated
@@ -219,6 +219,12 @@ exports.createFromJSON = function (obj, callback) {
}
}

exports.isPeerId = function (peerId) {
if (peerId.constructor && peerId.constructor.name) {
return peerId.constructor.name === 'PeerId'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do this I'm afraid, see libp2p/js-libp2p#65

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, it is even weird that this has only appeared now as an issue

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦆 all the way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish instanceof would just work..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use symbols for this

const peerIdSymbol = Symbol('PeerId')

class PeerId {
  constructor () {
    this[peerIdSymbol] = true
  }

  static isPeerId (other) {
    return other && other[peerIdSymbol]
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw this comment after pushed my change. Seems that Symbols won't work since they are different if different instances are created:

image

Making two versions (even patch) of peer-id different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the point of symbols, you reuse the same symbol on all instances as you can see above in my example

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You won't reuse if there is a js-peer-id 0.n.x and a 0.n+1.x.

@daviddias
Copy link
Member Author

Issue mitigated, will move forward with this PR. Will leave libp2p/js-libp2p#65 open for ideas.

@daviddias daviddias merged commit f088660 into master Mar 27, 2017
@daviddias daviddias deleted the feat/isPeerId branch March 27, 2017 13:17
@daviddias daviddias removed the status/in-progress In progress label Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants