Support Node 10 API
Closed this issue · 8 comments
I am ready to implement all of the changes in this library and do major refactor but @lukechilds seems to be taking too much time merging PRs. This library also has many problems:
- messed up codebase. super hard to understand
- got.stream when used in conjunction with cacheable request, the cached response always is buffer because how the responselike library used by this cacheable-request handles it poorly: https://github.com/lukechilds/responselike/blob/master/src/index.js
- minor caching bugs here and there which I don't exactly remember.
As a result I have created a fork: https://github.com/gumlet/cacheable-request
I maintain it and if GOT wishes, they can migrate to that module: https://www.npmjs.com/package/improved-cacheable-request
I love GOT and I am ready to implement the required changes in my library for GOT v10. If nothing works, at least provide an option in GOT with which, we can easily switch the cacheable-request library.
messed up codebase. super hard to understand
I disagree :P It's not that hard 😉
got.stream when used in conjunction with cacheable request, the cached response always is buffer because how the responselike library used by this cacheable-request handles it poorly:
I don't think you can cache the response without downloading the body first...
I maintain it and if GOT wishes, they can migrate to that module
That won't be necessary. I'm gonna redesign this module once I've finished my current work on Got.
I love GOT and I am ready to implement the required changes in my library for GOT v10.
Well, you could override the request option for now...
@szmarczak It's hard compared to my redesigned codebase.
The problem is actually not the download part. There it's handled very well. The problem is that responselike module is designed in such a way that even if a cache adapter is able to return stream while retrieving from cache, the responselike module forbids it. Any cache must return buffer and then responselike module converts it to stream in ugly way.
It would be awesome if you redesign this module (actually I like the fact that you agree that re-design is necessary). Please let us know if any redesign is there in your mind. I (and my company) would love to offer our axe for any amount of work which can improve this module.
I will love if new GOT works perfectly fine with streams including caching. Our use-case is that our responses must be streams, whether they are cached or not. Basically the files we are requesting are huge so it's always preferred to keep stream rather than convert to buffer.
the responselike module forbids it
You can use to-readable-stream.
I (and my company) would love to offer our axe for any amount of work which can improve this module.
I'll post here when I start :)
@szmarczak let me know if you are ready to start. I am sorry to be pushy but this library is super critical for our project so want to get it improved as soon as possible,
closing this out as we are now working to support Node 12, 14, 16, and 18. Should be updated in the next 90 days.