-
Notifications
You must be signed in to change notification settings - Fork 161
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
Add array stream decoding #42
Conversation
Codecov Report
@@ Coverage Diff @@
## master #42 +/- ##
==========================================
- Coverage 95.46% 91.46% -4.01%
==========================================
Files 14 14
Lines 772 808 +36
Branches 163 170 +7
==========================================
+ Hits 737 739 +2
- Misses 15 49 +34
Partials 20 20
Continue to review full report at Codecov.
|
Looks great! BTW Can it handle unlimited data stream? For example, something like push-notification via WebSocket that emits an item for each server-side event. GraphQL subscriptions are exactly the case. |
Here is a demo for streaming deserialization of Ruby's require "msgpack"
stream = StringIO.new([
# each item is an independent, complete MessagePack item
MessagePack.pack(true),
MessagePack.pack(42),
MessagePack.pack("foo"),
MessagePack.pack("bar"),
].join(""))
u = MessagePack::Unpacker.new(stream)
u.each do |obj|
p obj
end It emits |
I see, no this is one is specifically for arrays, but I can add version what will support this kind of streams. It's pretty easy to do. But let's do it in separate PR, because it's different from this one. |
Here is the version for decoding stream of objects async *decodeStream(stream: AsyncIterable<ArrayLike<number> | Uint8Array>) {
for await (const buffer of stream) {
this.appendBuffer(buffer);
try {
while (true) {
let result = this.decodeSync();
yield result;
}
} catch (e) {
if (!(e instanceof DataViewIndexOutOfBoundsError)) {
throw e; // rethrow
}
// fallthrough
}
}
} |
Good! Can you add And can you add tests to keep test coverage? |
Can I add tests in separate PR? :) I was going to start integrating this into our project today/tomorrow. |
Let's add decodeStream in separate PR too because I want to test it first. And just to keep each PR for separate functional increment. |
I will add tests for both decodeArrayStream and decodeStream during next days. |
btw, does Karma tests requires any additionally setup in this project?
all the time |
Nevermind. it's because of WebStrom. Works fine from terminal |
OK! So you can add tests and others in separate PRs. Will merge it and release a new version! |
BTW I use vscode to edit the code. Make sure |
Great! Wait for new version please. I need one more small change. I need to export Decoder and Encoder from index.ts. Are you good with it? I need to have access to raw decoder/encoder.
So I can't parse it using simple decode function. But it can be done by using Decoder directly |
#41