•  


Feature/streams by jonnyreeves · Pull Request #20 · jonnyreeves/chunked-request · GitHub
Skip to content
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

Feature/streams #20

Closed
wants to merge 3 commits into from
Closed

Feature/streams #20

wants to merge 3 commits into from

Conversation

jonnyreeves
Copy link
Owner

WIP implementation of stream-based parsers, see #16 .

@jonnyreeves jonnyreeves added this to the 0.6 milestone Sep 7, 2016
@jonnyreeves jonnyreeves self-assigned this Sep 7, 2016
@jonnyreeves
Copy link
Owner Author

cc @ariutta - sorry for the delay; hoping to start getting this nailed down now. I've gone for a slightly different API for the parser basing it on a trimmed down implementation of the Response object provided by the fetch API. This will give the parser access to the Headers, statusCode and provide the ability to cancel the response stream ( #19 )

Thanks to 
@jimmywarting
 for the inspiration; the `xhrTransport` now makes use of the `web-streams-polyfill` to return a `ReadableStream` which allows us to emulate the `Response.body()` method from the `fetch` standard.

Took this oppertunity to unify the xhr based implementations and make the transport detection be based on features rather than user-agent detection.
@jonnyreeves
Copy link
Owner Author

This branch now incorperate's @jimmywarting 's ideas on using a ReadableStream polyfill for the xhr-based transports; we can now provide a 'fetch-like' API to the parser function.

One question that arises from this change is does providing a parser make sense, or should the library just become a fetch-binary-stream which focuses on allowing you to make http requests from the browser for streamed binary data from the server?

This was referenced Sep 17, 2016
@jimmywarting
Copy link

jimmywarting commented Sep 17, 2016

You are kind of polyfilling the fetch api now? ??
Which has already been done: here
But they don't seem to plan on adding streaming support JakeChampion/fetch#198 ??

You should maybe make a PR to them :P

@jimmywarting
Copy link

jimmywarting commented Sep 17, 2016

I don't think you should parse the arrayBuffer to string. If it's going to work like fetch ReadableStream then parsing the response to text is, well kind of unexpected. The Stream api have something like a binary stream that is more friendly with memory allocation. Other libraries could take advantage of this such as jszip where they could "read into a ArrayBuffer allocation"

The TextDecoder api will eventually have some Transform stream
Then you just have to .pipeThrogh(new TextDecoder) something like that

You can create another module that's more like a polyfill out of TextDecoder transform. I almost did it with web-byline
You can take out most of the logic from index.js and only keep the TextDecoder part

Beside you don't want to parse a binary and you only get one ReadableStream back, so how do you know if you should parse it? you don't want to do it with a image... and it don't look good with an option such as createReadableStream({parse: boolean})

Think it's just better to pipe it
So i say remove the parser!

@jonnyreeves
Copy link
Owner Author

OK - taken a very different approach over the last few days and have pulled the guts of this library out into a new library: https://github.com/jonnyreeves/fetch-readablestream

I am unsure what to do about this library - it feels a touch redundant now as it just provides a defaultParser implementation which is trivial to re-create in user code, eg: https://github.com/jonnyreeves/fetch-readablestream/blob/release/v0.1.0/test/integ/util.js

Sign up for free to join this conversation on GitHub . Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants
- "漢字路" 한글한자자동변환 서비스는 교육부 고전문헌국역지원사업의 지원으로 구축되었습니다.
- "漢字路" 한글한자자동변환 서비스는 전통문화연구회 "울산대학교한국어처리연구실 옥철영(IT융합전공)교수팀"에서 개발한 한글한자자동변환기를 바탕하여 지속적으로 공동 연구 개발하고 있는 서비스입니다.
- 현재 고유명사(인명, 지명등)을 비롯한 여러 변환오류가 있으며 이를 해결하고자 많은 연구 개발을 진행하고자 하고 있습니다. 이를 인지하시고 다른 곳에서 인용시 한자 변환 결과를 한번 더 검토하시고 사용해 주시기 바랍니다.
- 변환오류 및 건의,문의사항은 juntong@juntong.or.kr로 메일로 보내주시면 감사하겠습니다. .
Copyright ⓒ 2020 By '전통문화연구회(傳統文化硏究會)' All Rights reserved.
 한국   대만   중국   일본