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

got 12.4.1 dependency cacheable-request does not transpile #2129

Closed
2 tasks done
AaronSterlingGENEICD opened this issue Sep 2, 2022 · 23 comments · Fixed by #2146
Closed
2 tasks done

got 12.4.1 dependency cacheable-request does not transpile #2129

AaronSterlingGENEICD opened this issue Sep 2, 2022 · 23 comments · Fixed by #2146

Comments

@AaronSterlingGENEICD
Copy link

AaronSterlingGENEICD commented Sep 2, 2022

Describe the bug

Running TypeScript 4.8.2:
Many errors generated on build of form
node_modules/@types/cacheable-request/index.d.ts:118:60 - error TS2709: Cannot use namespace 'ResponseLike' as a type.

npm explain latest-version
cacheable-request@7.0.2 dev
node_modules/cacheable-request
cacheable-request@"^7.0.2" from got@12.4.1
node_modules/got
got@"^12.1.0" from package-json@8.1.0
node_modules/package-json
package-json@"^8.1.0" from latest-version@7.0.0
node_modules/latest-version
dev latest-version@"^7.0.0" from the root project

Problem remediated with:

"overrides": {
    "latest-version": {
      "got": "12.3.1"
    }
  }

and installing with
npm ci
if installing with yarn, use --frozen-lockfile as noted by proton-ab below.

Checklist

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

This is caused by https://github.com/sindresorhus/responselike/releases/tag/v3.0.0 which is breaking for https://github.com/jaredwray/cacheable-request due to removal of @types/responselike and renaming of ResponseLike type to Response.

@h-unterp
Copy link

h-unterp commented Sep 3, 2022

Yep. Wasted an hour tracking this down. Hardcoded

"got": "12.3.0",

mydea added a commit to fabscale/ember-graphql-client that referenced this issue Sep 5, 2022
Due to sindresorhus/got#2129
Also pin the `@types/responselike` dependency to 1.0.0.
Some transitive deps are pinned to ensure we have somewhat consistent dependencies there...
@Rehan-05
Copy link

Rehan-05 commented Sep 6, 2022

Still facing error

mrgrain added a commit to projen/projen that referenced this issue Sep 6, 2022
@proton-ab
Copy link

proton-ab commented Sep 6, 2022

Recommendation for everyone to pin @types/responselike for now.

  "resolutions": {
    "@types/responselike": "1.0.0"
  },

Unless using frozen lockfile, you WILL fetch stub "@types/responselike": "3.0.0" because thoughtfully folks at DefinitelyTyped decided for @types/cacheable-request to depend on "@types/responselike": "*". This will break your application/package regardless of whether you updated or even installed got or not (but you are using --frozen-lockfile on production, right?).

@BendingBender
Copy link

BendingBender commented Sep 6, 2022

This is currently a known problem with DT: DefinitelyTyped/DefinitelyTyped#62111. You can workaround it by explicitly installing the older versions of the types packages:

npm i -D @types/cacheable-request@6.0.2 @types/responselike@1.0.0

If you use npm >8, you can instead add the following entry to your package.json:

"overrides": {
  "@types/responselike": "1.0.0"
}

If you use yarn, you can instead workaround this problem via the following entry in your package.json:

"resolutions": {
  "@types/responselike": "1.0.0"
}

Currently, the only way to really provide a fix is for got to explicitly depend on @types/responselike@1.0.0 or to upgrade the cacheable-request dependendency to cacheable-request@^9 or later.

@BendingBender
Copy link

BendingBender commented Sep 6, 2022

Another quick fix idea: include ambient types definitions from @types/responselike@1.0.0 directly in this lib via a .d.ts file:

declare module 'responselike' {
  // copy-paste types from @types/responselike@1.0.0
}

@HansBrende
Copy link

@BendingBender as of npm 8.3.0, I believe you can do the same thing with "overrides".

@BendingBender
Copy link

Ok, the DTS maintainers now have moved the latest tag back to @types/responselike@1.0.0. Plz let me know if that fixes your problems.

@AaronSterlingGENEICD
Copy link
Author

Thank you @BendingBender . If someone in the thread builds successfully with the latest version of got, please post. We won't run a build with an affected library until early next week. If I'm the first to report, I'll close this thread when our build succeeds.

@proton-ab
Copy link

proton-ab commented Sep 9, 2022

This will never work, regardless of DTS fixing it. got@12.4.1 depends on "responselike": "^3.0.0" but it also depends on "cacheable-request": "^7.0.2", which depends (and expects) "responselike": "^2.0.0". Note that responselike@3.0.0 is breaking due to ResponseLike => Respone type rename.

Taking a deeper dive, for TS cacheable-request@7.0.2 uses "@types/cacheable-request": "^6.0.2" which has following code:

import ResponseLike = require('responselike');

TypeScript compiler will hence look at resposelike/index.d.ts (ignoring @types/responselike/index.d.ts due to original package types taking precedence) and never find ResponseLike type there (as it is named Response).

Further, the only version of responselike installed by NPM/Yarn is 3.0.0 because got depends on it and ^2.0.0 of cacheable-request satisfies this requirement implicitly.

@sindresorhus You claim to support TS and ESM yet your own packages create a dependency conflict Right now got in versions above 12.4.0 has dependency conflict (even if it isn't detected by package manager due to NPM versioning scheme deficiency). for TypeScript users. You have two options:

  • Have got depend on responselike@2.0.1
  • Create responselike version 4.0.0 and amend type name to be same as one provided by DTS (ie. ResponseLike).

@jaredwray
You were put in a bad situation and forced to bear some of result of got new release. Your choices are simple:

  • Do nothing (there is no fault in cacheable-request
  • Have cacheable-request depend on responselike@3.0.0 (which should be simple and non-breaking you already depend on Node>=14)

@sindresorhus
Copy link
Owner

You claim to support TS and ESM yet your own packages create a dependency conflict. Right now got in versions above 12.4.0 has dependency conflict.

ESM works fine. This is a problem with TS.

even if it isn't detected by package manager due to NPM versioning scheme deficiency

npm works how it has always worked. This is a TS deficiency.

@sindresorhus
Copy link
Owner

I plan to upgrade cacheable-request, which will fix problem, but they need to make a new release first.

@AaronSterlingGENEICD
Copy link
Author

@sindresorhus Since you're in this thread, I'll take the opportunity to say: I really appreciate everything you've done for the community. A pleasure to interact at last.

@jaredwray
Copy link

@sindresorhus thanks for all the help on this and we have published v10.0.0 of cacheable-request. Please let me know if you have any issues with it as there are some breaking changes.

Release Notes for v9: https://github.com/jaredwray/cacheable-request/releases/tag/v9.0.0
Release Notes for v10: https://github.com/jaredwray/cacheable-request/releases/tag/v10.0.0

@proton-ab
Copy link

npm works how it has always worked. This is a TS deficiency.

Sorry I have forgot that conflicts with transitive dependencies are properly resolved here. I'll amend my original comment.

@sindresorhus
Copy link
Owner

Release Notes for v10: https://github.com/jaredwray/cacheable-request/releases/tag/v10.0.0

@jaredwray It would be great if the changelog included what it looked like before, not just the new way to call it. Right now it's very hard to see what needs changing. Example: https://github.com/sindresorhus/got/releases/tag/v12.0.0-beta.1 (see the examples with the diff syntax).

@sindresorhus
Copy link
Owner

I probably won't be able to get to this for a couple of weeks. I'm traveling and have limited time, and I thought this would be a simple upgrade, but it doesn't seem so.

Pull request welcome if anyone wants it to happen sooner. Need to change stuff around:

got/source/core/index.ts

Lines 971 to 1015 in f0ac0b3

private _prepareCache(cache: string | CacheableRequest.StorageAdapter) {
if (!cacheableStore.has(cache)) {
cacheableStore.set(cache, new CacheableRequest(
((requestOptions: RequestOptions, handler?: (response: IncomingMessageWithTimings) => void): ClientRequest => {
const result = (requestOptions as any)._request(requestOptions, handler);
// TODO: remove this when `cacheable-request` supports async request functions.
if (is.promise(result)) {
// We only need to implement the error handler in order to support HTTP2 caching.
// The result will be a promise anyway.
// @ts-expect-error ignore
// eslint-disable-next-line @typescript-eslint/promise-function-async
result.once = (event: string, handler: (reason: unknown) => void) => {
if (event === 'error') {
(async () => {
try {
await result;
} catch (error) {
handler(error);
}
})();
} else if (event === 'abort') {
// The empty catch is needed here in case when
// it rejects before it's `await`ed in `_makeRequest`.
(async () => {
try {
const request = (await result) as ClientRequest;
request.once('abort', handler);
} catch {}
})();
} else {
/* istanbul ignore next: safety check */
throw new Error(`Unknown HTTP2 promise event: ${event}`);
}
return result;
};
}
return result;
}) as typeof http.request,
cache as CacheableRequest.StorageAdapter,
));
}
}

@sindresorhus
Copy link
Owner

@jaredwray Also, cacheable-request has no code in the package when installed.

BastianBlokland added a commit to BastianBlokland/typedtree-editor that referenced this issue Sep 11, 2022
Newer version of 'got' (which is brought in by 'npm-check-updates')
depends on an outdated "@types/cacheable-requests"

More info: sindresorhus/got#2129
@jaredwray
Copy link

Release Notes for v10: https://github.com/jaredwray/cacheable-request/releases/tag/v10.0.0

@jaredwray It would be great if the changelog included what it looked like before, not just the new way to call it. Right now it's very hard to see what needs changing. Example: https://github.com/sindresorhus/got/releases/tag/v12.0.0-beta.1 (see the examples with the diff syntax).

This has been updated with a diff and we will look at how to help on this with a pull request.

@jaredwray
Copy link

@jaredwray Also, cacheable-request has no code in the package when installed.

cacheable-request version 10.0.1 is now released with code in the package. Sorry for the issue on that. This also contains some minor updates to keyv, normalize-url, and a couple others.

https://github.com/jaredwray/cacheable-request/releases/tag/v10.0.1

mydea added a commit to fabscale/ember-cognito-identity that referenced this issue Sep 12, 2022
Due to sindresorhus/got#2129 Also pin the `@types/responselike` dependency to 1.0.0. Some transitive deps are pinned to ensure we have somewhat consistent dependencies there...
@jaredwray
Copy link

We have now updated cacheable-request to export types which we believe will help with the migration to latest. Next, we will be looking at building a pull request in got. FYI

https://github.com/jaredwray/cacheable-request/releases/tag/v10.0.2

@alexkvak
Copy link

@sindresorhus have you plan to port this fix to got@11?

@sindresorhus
Copy link
Owner

have you plan to port this fix to got@11?

No

mergify bot pushed a commit to projen/projen that referenced this issue Nov 2, 2022
…2164)

Fixes: #2163

The underlying issue (sindresorhus/got#2129) has been resolved and packages are again building correctly with the latest versions of the affected packages. This PR serves as proof of this.

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
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

Successfully merging a pull request may close this issue.

9 participants