This repository has been archived by the owner on Mar 10, 2020. It is now read-only.
refactor: async iterables #567
Merged
Conversation
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
8 tasks
hugomrdias
suggested changes
Dec 9, 2019
This was referenced Dec 9, 2019
alanshaw
force-pushed
the
refactor/async-iterables
branch
from
f3aec8a
to
2af960a
Compare
lidel
suggested changes
Dec 13, 2019
There was a problem hiding this comment.
See my concern in ipfs-inactive/js-ipfs-http-client#1183 (review)
alanshaw
force-pushed
the
refactor/async-iterables
branch
2 times, most recently
from
186d007
to
1a568c9
Compare
lidel
reviewed
Dec 17, 2019
| @@ -129,14 +126,14 @@ A great source of [examples][] can be found in the tests for this API. | |||
|
|
|||
| | Type | Description | | |||
| | -------- | -------- | | |||
| | `Promise<Buffer>` | A Buffer with the data that the MerkleDAG node contained | | |||
| | `Promise<Buffer>` | An Promise that resolves to Buffer objects with the data that the MerkleDAG node contained | | |||
There was a problem hiding this comment.
Suggested change
| | `Promise<Buffer>` | An Promise that resolves to Buffer objects with the data that the MerkleDAG node contained | | |
| | `Promise<Buffer>` | A Promise that resolves to Buffer objects with the data that the MerkleDAG node contained | |
SPEC/PIN.md
Outdated
| | `Promise<Array>` | An array of current pinned objects | | ||
|
|
||
| an array of objects with keys `hash` and `type` is returned. | ||
| | `AsyncIterable<{ hash: string, type: string }>` | An async iterable that yields currently pinned objects with `hash` and `type` properties. `hash` is a string CID of the pinned node, `type` is the pin type ("recursive", "direct" or "indirect") | |
There was a problem hiding this comment.
I think hash should be replaced with cid, to match ipfs-inactive/js-ipfs-http-client#1183:
BREAKING CHANGE: pin.ls results now contain a cid property (a CID instance) instead of a string hash property.
| const output = await ipfs.cat(cidv1) | ||
| expect(output).to.eql(input) | ||
| const output = await concat(ipfs.cat(cidv1)) | ||
| expect(output.slice()).to.eql(input) |
There was a problem hiding this comment.
Why we need a shallow copy here (.slice())?
There was a problem hiding this comment.
concat returns a BufferList
alanshaw
force-pushed
the
refactor/async-iterables
branch
from
1890faa
to
7f39df2
Compare
achingbrain
reviewed
Jan 17, 2020
| * [resolve](#resolve) | ||
|
|
||
| ### ⚠️ Note | ||
| Although not listed in the documentation, all the following APIs that actually return a **promise** can also accept a **final callback** parameter. | ||
|
|
||
| #### `id` | ||
|
|
||
| > Returns the identity of the Peer |
There was a problem hiding this comment.
Suggestion: the returned id should be a rich object:
The returned object contains the following keys:
- `id` (CID) - A v1 [CID][cid] with the codec 'libp2p-key'
- `publicKey` (Buffer) - Bytes that make up the node's public key
- `addresses` (Array) - List of [Multiaddr][multiaddr] instances
- `agentVersion` (String) - An implementation type identifier
- `protocolVersion` (String) - An implementation version identifier
alanshaw
force-pushed
the
refactor/async-iterables
branch
from
6f0d978
to
d474125
Compare
alanshaw
pushed a commit
to ipfs-inactive/js-ipfs-http-client
that referenced
this pull request
Jan 23, 2020
TLDR;
* Remove Node.js streams and pull streams
* Remove callbacks
* Remove `peer-info` and `peer-id`
---
Now that internals are all async/await/iterables and `fetch` the next step is to bubble up that goodness to the core API, removing the multiple stream APIs and removing callback support.
I'm also proposing removing `peer-info` and `peer-id`, since these drastically increase the bundle size by pulling in `libp2p-crypto`, for which 99% of it's capability is unused. In place of `peer-id` we return a `CID`, which can easily be converted to a `PeerId` instance via:
```js
const peerId = PeerId.createFromCID(peerCid)
```
In place of `peer-info` we return an object `{ id: CID, addrs: Multiaddr[] }`, which can easily be converted to a `PeerInfo` like:
```js
const peerInfo = new PeerInfo(PeerId.createFromCID(info.id))
info.addrs.forEach(addr => peerInfo.multiaddrs.add(addr))
```
refs ipfs/js-ipfs#1670
refs ipfs/js-ipfs#2611
refs ipfs-inactive/interface-js-ipfs-core#394
TODO:
* [x] Refactor local tests
* [x] Remove `addFromFs` and `addFromUrl` and export `globSource` and `urlSource` instead
* [x] Refactor `interface-ipfs-core` tests
* [x] Document new APIs in `interface-ipfs-core`
* [x] Update README with API changes
* [x] [Document upgrade path from Node.js/pull streams to async iterables](https://gist.github.com/alanshaw/04b2ddc35a6fff25c040c011ac6acf26)
Depends on:
* [x] ipfs/js-ipfs-utils#15
* [x] ipfs-inactive/interface-js-ipfs-core#567
BREAKING CHANGE: Callbacks are no longer supported on any API methods. Please use a utility such as [`callbackify`](https://www.npmjs.com/package/callbackify) on API methods that return Promises to emulate previous behaviour.
BREAKING CHANGE: `PeerId` and `PeerInfo` classes are no longer statically exported from `ipfs-http-client` since they are no longer used internally.
BREAKING CHANGE: `pin.add` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.
BREAKING CHANGE: `pin.ls` now returns an async iterable.
BREAKING CHANGE: `pin.ls` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.
BREAKING CHANGE: `pin.rm` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.
BREAKING CHANGE: `add` now returns an async iterable.
BREAKING CHANGE: `add` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property.
BREAKING CHANGE: `addReadableStream`, `addPullStream` have been removed.
BREAKING CHANGE: `ls` now returns an async iterable.
BREAKING CHANGE: `ls` results now contain a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property.
BREAKING CHANGE: `files.ls` now returns an async iterable.
BREAKING CHANGE: `files.readPullStream` and `files.readReadableStream` have been removed.
BREAKING CHANGE: `files.read` now returns an async iterable.
BREAKING CHANGE: `files.lsPullStream` and `files.lsReadableStream` have been removed.
BREAKING CHANGE: `files.ls` now returns an async iterable.
BREAKING CHANGE: `files.ls` results now contain a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property.
BREAKING CHANGE: `files.ls` no longer takes a `long` option (in core) - you will receive all data by default.
BREAKING CHANGE: `files.stat` result now contains a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property.
BREAKING CHANGE: `get` now returns an async iterable. The `content` property value for objects yielded from the iterator is now an async iterable that yields [`BufferList`](https://github.com/rvagg/bl) objects.
BREAKING CHANGE: `stats.bw` now returns an async iterable.
BREAKING CHANGE: `addFromStream` has been removed. Use `add` instead.
BREAKING CHANGE: `isIPFS` is no longer exported from the client, please `npm i is-ipfs` or include the CDN script tag `<script src="https://unpkg.com/is-ipfs/dist/index.min.js"></script>` to use this utility in your applications.
BREAKING CHANGE: `addFromFs` has been removed. Please use the exported `globSource` utility and pass the result to `add`. See the [glob source documentation](https://github.com/ipfs/js-ipfs-http-client#glob-source) for more details and an example.
BREAKING CHANGE: `addFromURL` has been removed. Please use the exported `urlSource` utility and pass the result to `add`. See the [URL source documentation](https://github.com/ipfs/js-ipfs-http-client#url-source) for more details and an example.
BREAKING CHANGE: `name.resolve` now returns an async iterable. It yields increasingly more accurate resolved values as they are discovered until the best value is selected from the quorum of 16. The "best" resolved value is the last item yielded from the iterator. If you are interested only in this best value you could use `it-last` to extract it like so:
```js
const last = require('it-last')
await last(ipfs.name.resolve('/ipns/QmHash'))
```
BREAKING CHANGE: `block.rm` now returns an async iterable.
BREAKING CHANGE: `block.rm` now yields objects of `{ cid: CID, error: Error }`.
BREAKING CHANGE: `dht.findProvs`, `dht.provide`, `dht.put` and `dht.query` now all return an async iterable.
BREAKING CHANGE: `dht.findPeer`, `dht.findProvs`, `dht.provide`, `dht.put` and `dht.query` now yield/return an object `{ id: CID, addrs: Multiaddr[] }` instead of a `PeerInfo` instance(s).
BREAKING CHANGE: `refs` and `refs.local` now return an async iterable.
BREAKING CHANGE: `object.data` now returns an async iterable that yields `Buffer` objects.
BREAKING CHANGE: `ping` now returns an async iterable.
BREAKING CHANGE: `repo.gc` now returns an async iterable.
BREAKING CHANGE: `swarm.peers` now returns an array of objects with a `peer` property that is a `CID`, instead of a `PeerId` instance.
BREAKING CHANGE: `swarm.addrs` now returns an array of objects `{ id: CID, addrs: Multiaddr[] }` instead of `PeerInfo` instances.
BREAKING CHANGE: `block.stat` result now contains a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `key` property.
BREAKING CHANGE: `bitswap.wantlist` now returns an array of [CID](https://github.com/multiformats/js-cid) instances.
BREAKING CHANGE: `bitswap.stat` result has changed - `wantlist` and `peers` values are now an array of [CID](https://github.com/multiformats/js-cid) instances.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One day
it-allwill be https://tc39.es/proposal-iterator-helpers/#sec-asynciteratorprototype-toarray 🤞See the breaking changes messages in ipfs-inactive/js-ipfs-http-client#1183 for a high level overview of what is being changed.
TODO:
addFromFsandaddFromURLintoaddtests usingglobSourceandurlSourcefromipfs-utilsresolves #394