Too difficult to use decodeStream in browser #152

Activity

1. gfx commented on Feb 5, 2021

   

   gfx

   on Feb 5, 2021

   Last edited by gfx

   Member

   More actions

   Thanks for filing this issue. It sounds like a good viewpoint for me. I'd like to add examples to explain how to use decodeStream() in browsers.
2. Bilge commented on Feb 5, 2021

   

   Bilge

   on Feb 5, 2021

   Last edited by Bilge

   Author

   More actions

   I managed to use it as follows.

           let unpacked = [];
           for await (const item of decodeStream(new ReadableStream({
               pull: controller => {
                   controller.enqueue([...packed].map(s => s.charCodeAt(0)));
                   controller.close();
               }
           }))) {
               unpacked.push(item);
           }

   However, this seems highly convoluted and forces my entire decode branch to become an async call graph in what is otherwise a synchronous application. An alternative, synchronous API would be preferred. Recently, a similar feature was added to msgpackr.
3. gfx commented on Feb 6, 2021

   

   gfx

   on Feb 6, 2021

   Last edited by gfx

   Member

   More actions

   As I explain in #153, converting a binary to StreamLike is very easy if you are familiar with AsyncGenerator. Just three lines of code:

   declare var encoded: Uint8Array;
   
   const createStream = async function* (): AsyncGenerator<Uint8Array> {
     yield encoded;
   };
   
   for await (const item of decodeStream(createStream())) {
     // do something with item
   }

   What do you think of it?
4. Bilge commented on Feb 6, 2021

   

   Bilge

   on Feb 6, 2021

   Author

   More actions

   さすが、FUJIさんのコードの方が単純ですね！しかしUint8Arrayが必要ではありません。例えばnumber[]も良いですよ。

   （日本語が下手ならごめんなさい）
5. gfx commented on Feb 8, 2021

   

   gfx

   on Feb 8, 2021

   Last edited by gfx

   Member

   More actions

   @Bilge ありがとう 😉

   As you said, the exact type is ArrayLike<number>, as the reference describes.

   The difference between decodeMulti() (even if it doesn't exist) vs decodeStream() is that decodeMulti() doesn't require async function, indicating it might be easier to use. However, I strongly recommend using the async version. Async versions can use CPU and memory in a more effective way than sync ones.
6. prajaybasu commented on Feb 14, 2021

   

   prajaybasu

   on Feb 14, 2021

   More actions

   ArrayLike<number> does not feel completely suitable, Typescript is being really annoying for me when targeting browser. Code works otherwise.

   Argument of type 'ReadableStream<BufferSource>' is not assignable to parameter of type 'ReadableStreamLike<ArrayLike<number>>'.
     Type 'ReadableStream<BufferSource>' is not assignable to type 'ReadableStream<ArrayLike<number>>'.
       Type 'BufferSource' is not assignable to type 'ArrayLike<number>'.
         Property 'length' is missing in type 'ArrayBufferView' but required in type 'ArrayLike<number>'.ts(2345)
   

   Example code:

   async () => {
         const response = await fetch("items.msgpackgz");
         const gZipStream = new DecompressionStream("gzip");
         const dataSource = decodeArrayStream(
           response.body.pipeThrough(gZipStream) <--- DecompressionStream uses BufferSource, which doesn't work well with ArrayLike<number> 
         );
         return dataSource;
       }

   Probably related: microsoft/TypeScript#24195
7. Bilge commented on Feb 14, 2021

   

   Bilge

   on Feb 14, 2021

   Author

   More actions

   @gfx My only reason for mentioning it is that you might want to update the documentation in your PR with the "correct" type (ArrayLike<number>), just so it is clear to the reader.

   Anyway, since modern browsers seem to have good support for async and generators, this is no longer a major issue. It may still be difficult to use an async call in a non-async application for some people, though, because the entire call graph has to be converted to async to accommodate it.
8. gfx commented on Feb 14, 2021

   

   gfx

   on Feb 14, 2021

   Last edited by gfx

   Member

   More actions

   @prajaybasu Thank you for your report. I think it's irrelevant to this issue, so please file a separate issue. BTW I think it's actually insufficient for this library, so I'll fix it.

   @Bilge That's a good point. Conversion of sync functions to async functions is sometimes difficult or spending unnecessary time. Let me think again about decodeMulti().
9. gfx

   closed this as completedin #156on Feb 15, 2021
10. gfx commented on Feb 16, 2021

    

    gfx

    on Feb 16, 2021

    Member

    More actions

    Added decodeMulti() to v2.4.0. Thanks.

    👍React with 👍1Reacted by Bilge