jspm / npm

NPM Location Service
19 stars 31 forks source link

Add gzip compression support for npm lookups #133

Closed adamburgess closed 8 years ago

adamburgess commented 8 years ago

Something must've updated today on either NPM or Heroku's NPM cache, because it was forcing the use of gzip even when the http accept-encoding header explicitly disallowed gzip. So.... an easy fix would be to add gzip support to npm lookups. This should have 0 downsides (uncompressed requests are still supported) with lots of upsides (less data transfer!).

without this patch it seems that all installs through heroku are failing for me using the standard jspm install

adamburgess commented 8 years ago

Seems a lot of people are commenting about this at https://github.com/jspm/jspm-cli/issues/1679 - so this fix should probably be made against both master and 0.17...I'll make a PR

MTRo commented 8 years ago

Hi.

I fixed this issue.

node_modules/jspm-npm/npm.js (160)

  return asp(request)(auth.injectRequestOptions({
    uri: self.registryURL(scope) + '/' + repoPath,
    strictSSL: self.strictSSL,
    headers: lookupCache ? {
      'if-none-match': lookupCache.eTag
    } : {}
  }, doAuth && self.auth)).then(function(res) {

To this.

  return asp(request)(auth.injectRequestOptions({
    uri: self.registryURL(scope) + '/' + repoPath,
    strictSSL: self.strictSSL,
    gzip : true,
    headers: lookupCache ? {
      'if-none-match': lookupCache.eTag
    } : {}
  }, doAuth && self.auth)).then(function(res) {

Add "gzip : true" option for Http request of "request" module options.

MTRo commented 8 years ago

oh sorry. already fixed...

adamburgess commented 8 years ago

yeah, what you proposed is (almost) exactly my change — though it means we came to the same fix independently, which is good

kayue commented 8 years ago

@AdamBurgess Can you also push a fix to 0.16?

We are having trouble upgrading to 0.17...

Thanks a lot

adamburgess commented 8 years ago

@kayue #134 is based on master/0.16