This repository has been archived by the owner on Oct 19, 2022. It is now read-only.
refactor: use async / await #52
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
* feat: dream API for multistream-select
Here's some dream code! This is how the API could look in an async/await future. I would love to know what you think and if you have questions.
I **think** I have all my assumptions correct about how the current implementation works and is used 🤣!
* feat: select method
As per the dream code this function allows selecting a protocol from the passed list of protocols using the provided muxed stream.
We just iterate over the passed protocols and...
1. Write to the muxed stream sink with (length prefixed) protocol
2. Read response from muxed stream source
* We use `it-reader` and `it-length-prefixed` and read single bytes until we have the whole response
* I understand now why `pull-length-prefixed` has a `decodeFromReader` function and I'll add that to `it-length-prefixed` and use it here
* TLDR; We only need to read single bytes until we have a valid varint!
3. Check if response is expected protocol (not "na")
4. If expected then return stream and protocol, if not then iterate
The interesting bit is where we construct the `selectedStream` (the stream we return to the caller). We need to use the sink from the muxed stream to write our multistream protocol(s) and _then_ we need to allow it to be used by whoever needs it. This is just a concat of two sources, but the tricky bit is that the second is not known until after we return the selected stream to the caller. Line 20-24 pass a source to the muxed stream sink that iterates over our writer stream and then waits for a source to be provided to the `selectedStream` sink. The `selectedStream` sink function just resolves this promise and the content from this source is then yielded from the source we passed to the muxed stream's sink.
The source for the `selectedStream` is just the `reader`. You can think of it as just a wrapper around the muxed stream's source. It is used to read responses during multistream negotiation but can then continue to be used by whoever needs to read the rest of the stream. This is because it's just an iterable who's next function optionally takes a number of bytes to read. See the [example](https://github.com/alanshaw/it-reader#usage) for more info.
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
* feat: handle and ls
This PR supersedes #1 and #2 and adds `ls` and `handle` on top of `select` and adds 100% test coverage:
<img width="1281" alt="Screenshot 2019-08-14 at 12 21 52" src="https://user-images.githubusercontent.com/152863/63018364-89eba280-be90-11e9-833e-db5097b092c0.png">
API has changed a little from #2 because we actually do need to keep some state around for `ls`.
If you use `ls` you need to remember you've done the multistream-select handshake so that you can call `select` afterwards.
So the API is now:
```js
const dialer = new MSS.Dialer(muxedStream)
const protocols = await dialer.ls() // optional!
const { stream, protocol } = await dialer.select('/echo/1.0.0')
// --- and ---
const listener = new MSS.Listener(muxedStream)
const { stream, protocol } = await listener.handle('/echo/1.0.0')
```
This work also uncovered a possible bug in `go-multistream` multiformats/go-multistream#41
I also addressed feedback from #2 here.
If this is approved I'll open a PR to `js-multistream-select`.
closes #1
closes #2
cc @jacobheun @vasco-santos @daviddias
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
* fix: fixes and tests
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
* docs: remove ramblings from README
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
* docs: fix comment
Co-Authored-By: Jacob Heun <jacobheun@gmail.com>
* fix: listener and dialer selection results
Co-Authored-By: Jacob Heun <jacobheun@gmail.com>
Multistream spec: multiformats/multistream-select#19 fixes #4
vasco-santos
approved these changes
Sep 25, 2019
There was a problem hiding this comment.
This looks really great! Awesome work @alanshaw @jacobheun
Just left minor suggestions
|
@jacobheun dependencies updated - enjoy 👍 |
jacobheun
added a commit
that referenced
this pull request
Oct 10, 2019
BREAKING CHANGE: The API has changed from callbacks to async/await. Additionally, the use has changed. See the README for usage guidance. Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
jacobheun
pushed a commit
that referenced
this pull request
Oct 10, 2019
BREAKING CHANGE: The API has changed from callbacks to async/await. Additionally, the use has changed. See the README for usage guidance.
Merged
jacobheun
added a commit
that referenced
this pull request
Oct 11, 2019
* refactor: use async / await (#52) BREAKING CHANGE: The API has changed from callbacks to async/await. Additionally, the use has changed. See the README for usage guidance. * feat: consolidate select write (#53) * chore: update deps * chore: remove package lock * chore: update contributors * chore: release version v0.15.0 * chore: fix contributors
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.
This PR merges in the async/await refactor from https://github.com/alanshaw/it-multistream-select.
Notable changes from
it-multistream-selectare the repo references, example code referencing toit-mplexand updates to package.json and the readme to make them comply with the multistream-select repo.Note: This does currently reference a few github tags as dependencies, rather than creating temporary npm releases of the modules. Once the PRs are merged & released we can bump the versions here.