defer gzip handling to `request`

npm / npm-registry-client

http://npm.im/npm-registry-client

ISC License

264 stars 108 forks source link

defer gzip handling to `request` #133

Closed stefanpenner closed 8 years ago

stefanpenner commented 8 years ago

if not supported, no GZIP is returned. All is well
If supported, GZIP is returned but handled entirely by request and we get smaller payloads.

stefanpenner commented 8 years ago

oops missed local failing tests will look.

stefanpenner commented 8 years ago

It appears the meta-data requests already have the appropriate header: https://github.com/npm/npm-registry-client/pull/133/files#diff-96d9811cb2296e877edf463c6cfc57bdR101 I believe I'm pwning myself, my local nginx appears to encode paths gzip ending in / but ones without (what npm requests) are not.

I suspect once I address that, the meta-data will be correctly compressed. I'll report back and close once i confirm.

stefanpenner commented 8 years ago

That being said, request module already supports gzip via opt.gzip. It may be cleaner in the end to push all the inflating/deflating to request entirely. Unless someone recalls a reason why this is handled instead in this module...

stefanpenner commented 8 years ago

I have updated the PR to delegate all gzip to the request module. Their may have been a reason this module choose not to originally do this, but in-case it was an oversight, a wild PR appears.

zkat commented 8 years ago

Since we plan on replacing request altogether, we talked about this, and decided that it's probably better for us to keep this code around to make it easy to refer to, and it'll eventually just live somewhere else. So while I think this is a good patch, we probably won't take it this time around. Thanks a bunch for your time and care and research on this. <3