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

Dependency cacheable-request is outdated and does not distinguish POST requests with different bodies #2138

Closed
2 tasks done
AlCalzone opened this issue Sep 9, 2022 · 13 comments

Comments

@AlCalzone
Copy link
Contributor

Describe the bug

  • Node.js version: not relevant
  • OS & version: not relevant

Before version 8, cacheable-request does not correctly distinguish POST requests to the same URL if their body is different. This was fixed in https://github.com/jaredwray/cacheable-request/pull/31

I tried my hand at upgrading it, but due to changes to types that are used in got, this wasn't trivial for me. I'm sure someone who's more familiar with this codebase can do it in no time.

Actual behavior

cacheable-request is on version 7

Expected behavior

cacheable-request should be version 8 or 9

Code to reproduce

Not relevant, this is related to upgrading a dependency

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@jeswr
Copy link

jeswr commented Sep 14, 2022

In addition @types/cacheable-request should be removed from version 9+ (see DefinitelyTyped/DefinitelyTyped#62099). I've had some type errors occurring downstream that were caused by the presence of these types in my node modules.

@jaredwray
Copy link

This is being reviewed and looked at here: #2146 and tracked here: #2129

Please follow along on those.

@nbouvrette
Copy link

nbouvrette commented Sep 15, 2022

I just started having very odd issues related to cacheable-request. I'm still trying what triggered them, but got is the only dependency when I check with npm list. As per DefinitelyTyped/DefinitelyTyped#62099, this can be fixed by both upgrading and removing the types:

node_modules/@types/cacheable-request/index.d.ts:26:42 - error TS2709: Cannot use namespace 'ResponseLike' as a type.

26         cb?: (response: ServerResponse | ResponseLike) => void
                                            ~~~~~~~~~~~~

@jaredwray
Copy link

@nbouvrette - please see the plan above.

@nbouvrette
Copy link

@jaredwray Yes I've been reading several threads but for some reason, no workaround is working so far.

@jaredwray
Copy link

@nbouvrette - that is correct that there are no work arounds that I have seen so far. the goal is to update cacheable-request to latest in got which is in this pull request being worked on👇

#2146

@jaredwray
Copy link

Update to cacheable-request with the function requested change is now published: https://github.com/jaredwray/cacheable-request/releases/tag/v10.1.2

- const cacheableRequest = new CacheableRequest(https.request).createCacheableRequest();
+ const cacheableRequest = new CacheableRequest(https.request).request(); 

@nbouvrette
Copy link

@jaredwray if I am following, there got still needs to publish a new release using the latest cacheable-request in order to fix the issue?

@jaredwray
Copy link

@jaredwray if I am following, there got still needs to publish a new release using the latest cacheable-request in order to fix the issue?

Correct. It is being actively worked on

@nbouvrette
Copy link

nbouvrette commented Sep 16, 2022

For those interested I did find a workaround (requires npm v8.3.0 (2021-12-09)):

Update your package.json and add:

  "overrides": {
    "@types/cacheable-request": "8.3.1"
  },

Edit /node_modules/cacheable-request/dist/types.d.ts and remove the 4 first lines /// <reference types="node" resolution-mode="require"/> (they are triggering a 'resolution-mode' assertions are only supported when 'moduleResolution' is 'node16' or 'nodenext'. error and I could not find anything on Google... removing them worked for me)

@sindresorhus
Copy link
Owner

cacheable-request was updated in https://github.com/sindresorhus/got/releases/tag/v12.5.0

@AlCalzone
Copy link
Contributor Author

Unfortunately, this is only half-fixed. got does not pass the request body to cacheable-request. I'll raise a PR

@jaredwray
Copy link

@AlCalzone - thanks for jumping on this.

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

No branches or pull requests

5 participants