request / request-promise

The simplified HTTP request client 'request' with Promise support. Powered by Bluebird.
ISC License
4.76k stars 297 forks source link

Limit the tough-cookie to 2.5 as 3.x removes node 0.10 support #299

Closed aomdoa closed 5 years ago

aomdoa commented 5 years ago

Latest tough-cookie does not have node 0.10 support. Update the package.json to limit to the 2.5 version.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 145733887f18e0c3c4cc2b9cc226bf01ad231150 on aomdoa:fixToughCookieDep into 18c838a1ba2e201cdb263a3ec41e0e66453c9c9c on request:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 145733887f18e0c3c4cc2b9cc226bf01ad231150 on aomdoa:fixToughCookieDep into 18c838a1ba2e201cdb263a3ec41e0e66453c9c9c on request:master.

jrjohnson commented 5 years ago

Just a note that the latest tough-cookie version also drops node 6 which is not yet EOL. This bump to 3.0.0 should probably have been a breaking change in request-promise, but the >= constraint let it slip through.

analog-nico commented 5 years ago

Hey @aomdoa @jrjohnson quick question: My assumption is that tough-cookie is only used by your own code. Thus you will install it yourself in your project and with that define the version of the library that you use. request-promise is lenient here because it shall use the same version of tough-cookieas you use in your own code. Is my assumption right or did I miss something?

jrjohnson commented 5 years ago

I'm using neither tough-cookie or resolve-promise directly. My chain looks like

└─┬ ember-percy@1.5.0
  └─┬ percy-client@3.0.3
    ├─┬ request@2.88.0
    │ └── tough-cookie@2.4.3 
    └─┬ request-promise@4.2.2
      └── tough-cookie@3.0.1

The issue is that a breaking change of tough-cookie bubbled up through the chain to be a breaking change in my library because the version constraint >=2.3.3 allowed it through. In my experience it is pretty rare for a library like request-promise to have a >= version constraint, but it does put you in a position where you have to stay up to date with your dependencies and then manage new releases which is annoying.

analog-nico commented 5 years ago

Thank you very much @jrjohnson ! That makes sense. Well, ideally percy-client would take care of this but since this is surely not the only package where we run into it, it would be a bottomless pit to open issues for these projects.

To ensure that even possible edge cases are gone once you install the next release that I’ll do shortly, please check afterwards that request and request-promise don’t have separately installed versions of tough-cookie anymore and instead tough-cookie is physically only installed once.

analog-nico commented 5 years ago

request-promise@4.2.3 is released which includes this fix.

jrjohnson commented 5 years ago

Thanks @analog-nico! I agree that I could have avoided this (and did in the end) by just not updating my lock file so this transitive dependency update didn't hit my app. I'll check what is being installed when I next update. Would you like me to submit a PR that updates tough-cookies to the 3.0 series so it can be released here as a breaking 5.0?

analog-nico commented 5 years ago

Thanks again for your input @jrjohnson ! No need for a PR. Thanks. The important point here is that request and request-promise must use the exact same physical copy of the lib because it otherwise fails to do an importance instanceof comparison. Currently, request uses @~2.5.0 so tough-cookie@3 is not relevant yet.