-
Notifications
You must be signed in to change notification settings - Fork 44
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* refactor: make import and creation async - This allows the use of native key generation in the browser BREAKING CHANGE: This changes the interface of .create, .createFromPrivKey, .createFromPubKey, .createFromJSON
- Loading branch information
1 parent
e08907f
commit 31701e2
Showing
8 changed files
with
325 additions
and
167 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,8 @@ | |
"name": "peer-id", | ||
"version": "0.7.0", | ||
"description": "IPFS Peer Id implementation in Node.js", | ||
"main": "lib/index.js", | ||
"main": "src/index.js", | ||
"bin": "src/bin.js", | ||
"jsnext:main": "src/index.js", | ||
"scripts": { | ||
"lint": "aegir-lint", | ||
"build": "aegir-build", | ||
|
@@ -27,24 +26,25 @@ | |
"test" | ||
], | ||
"engines": { | ||
"node": "^4.3.0" | ||
"node": ">=4.0.0" | ||
}, | ||
"bugs": { | ||
"url": "https://github.com/diasdavid/js-peer-id/issues" | ||
"url": "https://github.com/libp2p/js-peer-id/issues" | ||
}, | ||
"homepage": "https://github.com/diasdavid/js-peer-id", | ||
"homepage": "https://github.com/libp2p/js-peer-id", | ||
"devDependencies": { | ||
"aegir": "^8.0.0", | ||
"aegir": "^9.0.1", | ||
"chai": "^3.5.0", | ||
"pre-commit": "^1.1.3" | ||
}, | ||
"dependencies": { | ||
"libp2p-crypto": "^0.6.1", | ||
"async": "^2.0.1", | ||
"libp2p-crypto": "^0.7.0", | ||
"multihashes": "^0.2.2" | ||
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/diasdavid/js-peer-id.git" | ||
"url": "https://github.com/libp2p/js-peer-id.git" | ||
}, | ||
"contributors": [ | ||
"David Dias <[email protected]>", | ||
|
@@ -53,4 +53,4 @@ | |
"dignifiedquire <[email protected]>", | ||
"nginnever <[email protected]>" | ||
] | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
31701e2
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.
What informed the decision to implement this with callbacks instead of modern promise style? Trying to merge in a greenkeeper update against this release and the amount of refactoring with callback style is more than seems necessary.
31701e2
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.
Ok, looking around this seems consistent with other usage in the codebase. My vote is still for promises 😄
31701e2
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.
@parkan yes this was not a new decision, just keeping in line with the rest of the code base. For more fun discussions around this please visit ipfs/js-ipfs#557 and weigh in
31701e2
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.
@dignifiedquire thanks for the explanation, I'll chime in there.
I tried to wrap these in promises, and I'm actually running into some trouble. Tried using
thenable
andes6-promisify
and in both cases I end up with promises that never complete (stuck in pending state and neither handler fires). The code is pretty simple and I've done this many times before, so not sure what could be the cause -- to further confuse things if I use thenable'swithCallback
and pass a callback to the transformed function it still works.Any idea why this might happen, or maybe an example of promise-wrapped use somewhere?
31701e2
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.
@parkan could you file an issue with some repro code please? Then it's easier for me to fix
31701e2
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.
Hrm can't reproduce this behavior anymore after shuffling around branches and npm updating, it's possible my modules were in a weird state. However, I noticed something else:
PeerId.create
does not check for the presence of a callback likecreateFromPubKey
etc do: https://github.com/libp2p/js-peer-id/blob/master/src/index.js#L88 so you can crash like so:31701e2
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.
I think a related issue (from a code style viewpoint) is the mixed callback/direct return style API with no way to tell which functions have which behaviors without examining the signature or getting the "callback required" warning.