got icon indicating copy to clipboard operation
got copied to clipboard

Got 12.1.0 x Typescript 4.7.2 x Node 16.15 transpilation fails with moduleResolution=Node16

Open ClementValot opened this issue 2 years ago • 15 comments

Describe the bug

  • Node.js version: 16.15.0
  • OS & version: Windows 11

Actual behavior

Requiring got and transpiling with Typescript 4.7 raises transpilation errors

node_modules/got/dist/source/core/options.d.ts(763,22): error TS2709: Cannot use namespace 'CacheableLookup' as a type.
node_modules/got/dist/source/core/options.d.ts(764,26): error TS2709: Cannot use namespace 'CacheableLookup' as a type.
node_modules/got/dist/source/core/options.d.ts(775,21): error TS2709: Cannot use namespace 'CacheableLookup' as a type.
node_modules/got/dist/source/core/options.d.ts(776,25): error TS2709: Cannot use namespace 'CacheableLookup' as a type.
node_modules/got/dist/source/core/options.d.ts(1125,29): error TS2709: Cannot use namespace 'CacheableLookup' as a type.

Code to reproduce

package.json

{
  "name": "got-ts4.7-regression",
  "version": "1.0.0",
  "type": "module",
  "dependencies": {
    "got": "^12.1.0"
  },
  "devDependencies": {
    "typescript": "^4.7.2"
  }
}

tsconfig.json

{
  "compilerOptions": {
    "module": "Node16",
    "moduleResolution": "Node16",
    "target": "es6",
  },
  "exclude": [
    "node_modules"
  ]
}

"moduleResolution": "Node16" is apparently required for Typescript 4.7 support of the exports field in package.json

Checklist

  • [x] I have read the documentation.
  • [x] I have tried my code with the latest version of Node.js and Got.

ClementValot avatar Jun 02 '22 18:06 ClementValot

Apparently we need to set module https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#ecmascript-module-support-in-node-js

szmarczak avatar Jun 02 '22 21:06 szmarczak

Same, a little bit confused here. I cannot get this package to compile with TypeScript after the switch to ESM. Setting skipLibCheck to true in compilerOptions is the only way to get this package to compile. Using TypeScript 4.7.3 and got 12.1.0.

I feel like I'm missing something, does anyone have a barebones repo with TypeScript working?

node_modules/got/dist/source/core/options.d.ts:14:35 - error TS7016: Could not find a declaration file for module 'form-data-encoder'. '/home/akowald/projects/shipping-library/node_modules/form-data-encoder/lib/esm/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/form-data-encoder` if it exists or add a new declaration (.d.ts) file containing `declare module 'form-data-encoder';`

14 import type { FormDataLike } from 'form-data-encoder';
                                     ~~~~~~~~~~~~~~~~~~~

node_modules/got/dist/source/core/options.d.ts:763:22 - error TS2709: Cannot use namespace 'CacheableLookup' as a type.

763     get dnsLookup(): CacheableLookup['lookup'] | undefined;
                         ~~~~~~~~~~~~~~~

node_modules/got/dist/source/core/options.d.ts:764:26 - error TS2709: Cannot use namespace 'CacheableLookup' as a type.

764     set dnsLookup(value: CacheableLookup['lookup'] | undefined);
                             ~~~~~~~~~~~~~~~

node_modules/got/dist/source/core/options.d.ts:775:21 - error TS2709: Cannot use namespace 'CacheableLookup' as a type.

775     get dnsCache(): CacheableLookup | boolean | undefined;
                        ~~~~~~~~~~~~~~~

node_modules/got/dist/source/core/options.d.ts:776:25 - error TS2709: Cannot use namespace 'CacheableLookup' as a type.

776     set dnsCache(value: CacheableLookup | boolean | undefined);
                            ~~~~~~~~~~~~~~~

node_modules/got/dist/source/core/options.d.ts:1125:29 - error TS2709: Cannot use namespace 'CacheableLookup' as a type.

1125         dnsCache: boolean | CacheableLookup | undefined;
                                 ~~~~~~~~~~~~~~~


Found 6 errors in the same file, starting at: node_modules/got/dist/source/core/options.d.ts:14

akowald avatar Jun 06 '22 20:06 akowald

To fix this, we need to upgrade AVA to v4 and then upgrade @sindresorhus/tsconfig to v3. Contributions welcome.

sindresorhus avatar Jul 05 '22 20:07 sindresorhus

I'm currently giving it a try, and upgrading @sindresorhus/tsconfig to 3.0.1 breaks a lot of type-checking, notably with default imports, e.g import is from '@sindresorhus/is'; raises TS2339: Property 'object' does not exist on type 'typeof import("D:/Projects/WebstormProjects/got/node_modules/@sindresorhus/is/dist/index")'.

Switching it for const is = require("@sindresorhus/is").default; fixes it but it does not feel natural, let alone ESM-compliant. What am I missing?

ClementValot avatar Jul 27 '22 06:07 ClementValot

I really feel like this issue should have a much higher priority. Almost all sindresorhus packages have been promoting ESM only for a long time, and now that TS finally have proper support, this issue alone prevents me from finally migrating a big TypeScript project to ESM.

Really hope someone picks this up soon, i will happily donate a cup of coffee to anyone moving on with this issue 😄 ❤️

Baune8D avatar Aug 10 '22 12:08 Baune8D

We hear you @Baune8D :heart: Unfortunately there's just only me and @sindresorhus. I'm quite busy with full-time work, I'll try to find time later this week to push this forward, but no promises. We encourage more people to make contributions :heart:

szmarczak avatar Aug 10 '22 18:08 szmarczak

So after upgrading @sindresorhus/tsconfig to 3.0.1, which gives moduleResolution=Node16, type checking breaks, notably on CacheableLookup, form-data-encoder and then-busboy

tsc --traceResolution gives an explanation:

======== Resolving module 'form-data-encoder' from 'D:/WebstormProjects/@ClementValot/got/source/core/options.ts'. ======== Explicitly specified module resolution kind: 'Node16'. File 'd:/webstormprojects/@clementvalot/got/source/core/package.json' does not exist according to earlier cached lookups. File 'd:/webstormprojects/@clementvalot/got/source/package.json' does not exist according to earlier cached lookups. File 'd:/webstormprojects/@clementvalot/got/package.json' exists according to earlier cached lookups. Loading module 'form-data-encoder' from 'node_modules' folder, target file type 'TypeScript'. Directory 'D:/WebstormProjects/@ClementValot/got/source/core/node_modules' does not exist, skipping all lookups in it. Directory 'D:/WebstormProjects/@ClementValot/got/source/node_modules' does not exist, skipping all lookups in it. Found 'package.json' at 'D:/WebstormProjects/@ClementValot/got/node_modules/form-data-encoder/package.json'. 'package.json' does not have a 'typesVersions' field. File name 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.js' has a '.js' extension - stripping it. File 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.ts' does not exist. File 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.tsx' does not exist. File 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.d.ts' does not exist. Directory 'D:/WebstormProjects/@ClementValot/node_modules' does not exist, skipping all lookups in it. Directory 'D:/WebstormProjects/node_modules' does not exist, skipping all lookups in it. Directory 'D:/node_modules' does not exist, skipping all lookups in it. File 'd:/webstormprojects/@clementvalot/got/source/core/package.json' does not exist according to earlier cached lookups. File 'd:/webstormprojects/@clementvalot/got/source/package.json' does not exist according to earlier cached lookups. File 'd:/webstormprojects/@clementvalot/got/package.json' exists according to earlier cached lookups. Loading module 'form-data-encoder' from 'node_modules' folder, target file type 'JavaScript'. Directory 'D:/WebstormProjects/@ClementValot/got/source/core/node_modules' does not exist, skipping all lookups in it. Directory 'D:/WebstormProjects/@ClementValot/got/source/node_modules' does not exist, skipping all lookups in it. File 'D:/WebstormProjects/@ClementValot/got/node_modules/form-data-encoder/package.json' exists according to earlier cached lookups. File name 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.js' has a '.js' extension - stripping it. File 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.js' exist - use it as a name resolution result. Resolving real path for 'd:/webstormprojects/@clementvalot/got/node_modules/form-data-encoder/lib/index.js', result 'D:/WebstormProjects/@ClementValot/got/node_modules/form-data-encoder/lib/index.js'. ======== Module name 'form-data-encoder' was successfully resolved to 'D:/WebstormProjects/@ClementValot/got/node_modules/form-data-encoder/lib/index.js' with Package ID 'form-data-encoder/lib/[email protected]'. ========

=> form-data-encoder has a package.json with a types field that points to @types/index.d.ts, but Node does not check that field with moduleResolution = Node16 or NodeNext and instead looks for a typesVersion field instead, which it doesn't find, and doesn't find the types declaration file in any of the default locations, hence no type resolution.

Same issue for then-busboy, something different for CacheableLookup, haven't figured that out yet Off to read some documentation around ModuleResolution to determine whether that's a node issue or a form-data-encoder issue

ClementValot avatar Aug 13 '22 17:08 ClementValot

Finally decided to move to ESM and now being blocked by this issue unfortunately. Hoping for the swift success of #2109.

dscalzi avatar Sep 02 '22 20:09 dscalzi

Someone will have to open a pull request on form-data-encoder and then-busboy before we can move forward with this. We have fixed everything we can in Got (https://github.com/sindresorhus/got/pull/2132).

sindresorhus avatar Sep 05 '22 03:09 sindresorhus

something different for CacheableLookup, haven't figured that out yet

// @szmarczak ^

sindresorhus avatar Sep 05 '22 03:09 sindresorhus

I have opened PRs for both form-data-encoder and then-busboy https://github.com/octet-stream/form-data-encoder/pull/10 https://github.com/octet-stream/then-busboy/pull/34

Baune8D avatar Sep 06 '22 08:09 Baune8D

I've released [email protected] and [email protected] with these fixes. Thanks @Baune8D!

octet-stream avatar Sep 06 '22 10:09 octet-stream

Now that https://github.com/sindresorhus/got/issues/2129 is closed, this is another issue that blocks got from being used in TypeScript with ESM because cacheable-request now requires node16 to be set due to:

node_modules/cacheable-request/dist/types.d.ts(1,23): error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.
node_modules/cacheable-request/dist/types.d.ts(2,23): error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.
node_modules/cacheable-request/dist/types.d.ts(3,23): error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.
node_modules/cacheable-request/dist/types.d.ts(4,23): error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.

Frankly, last few releases from @sindresorhus (not only Got) been a sick joke introducing more problems to people that chose to follow his advises and use ESM.

proton-ab avatar Sep 19 '22 22:09 proton-ab

I temporarily solved this issue by removing /// <reference types="node" resolution-mode="require"/> from node_modules/cacheable-request/dist/types.d.ts and running npx patch-package cacheable-request, see Maxim-Mazurok/google-api-typings-generator@c1f1654 (#703)

Maxim-Mazurok avatar Sep 20 '22 01:09 Maxim-Mazurok

The only blocking issue left for turning on strict mode here is https://github.com/szmarczak/cacheable-lookup having incorrect types. It uses default export without being ESM. I have asked @szmarczak if he can resolve this.

sindresorhus avatar Sep 20 '22 03:09 sindresorhus

I appreciate the push forward, but mainline releases really should be stable. The ESM requirement should have remained on a pre-release line until dependency issues were resolved. Yes, 11 is a fallback that most people use but new features all target 12.

dscalzi avatar Sep 22 '22 15:09 dscalzi

For what it’s worth:

I updated Got from 12.4.1 to 12.5.0, ran into this issue, and found this thread. For the time being, I downgraded back to 12.4.1 and everything is back to a working state.


Let me take the opportunity to thank y’all involved in Got and its dependencies! You’re the best!

I really appreciate your attitude of moving things forward (ESM, and that sort of thing). Even when I get a paper cut from the bleeding edge, it’s always easy to backtrack a little 😁

Let me know if you want more information about my specific scenario or if I can help test something!

leafac avatar Sep 23 '22 13:09 leafac

https://github.com/sindresorhus/got/releases/tag/v12.5.1

sindresorhus avatar Sep 27 '22 07:09 sindresorhus

@sindresorhus: Thanks for the update!

I updated to 12.5.1 but I’m still getting errors of the following form:

node_modules/got/dist/source/core/response.d.ts:1:23 - error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.

1 /// <reference types="node" resolution-mode="require"/>

Naturally, my tsconfig.json sets "moduleResolution": "node". I tried changing it to node16 or nodenext and the errors from Got do get resolved, but then a bunch of other packages yield errors, for example:

node_modules/hast-util-raw/index.d.ts:3:26 - error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

3 export type Raw = import('./complex-types').Raw
                           ~~~~~~~~~~~~~~~~~

node_modules/hast-util-raw/lib/index.d.ts:19:43 - error TS2694: Namespace '"/Users/leafac/Code/courselore/node_modules/parse5/dist/index"' has no exported member 'Document'.

19 export type P5Document = import('parse5').Document
                                             ~~~~~~~~

So now I don’t know what’s the right way forward:

Possibility 1: I should continue with "moduleResolution": "node" but there’s still more things that need to change in Got.

Possibility 2: I should switch to node16 or nodenext but there are still other packages that need changes. In that case I should talk to them about it…

By the way, I’m fine either way. It could just like the RequireJS → ESM transition in which it takes a bit of time & work, but everyone’s better off in the end 😁

I just need a bit guidance on what to do next…

Thanks again 👏

leafac avatar Sep 27 '22 11:09 leafac

I would go with 2. In the future, more packages are switching to node16, so if you choose 1, you're just going to hit this problem with another package.

Alternatively, you could maybe set "skipLibCheck": true in your tsconfig.

sindresorhus avatar Sep 27 '22 11:09 sindresorhus

Sounds good. Thanks for the information 😁

leafac avatar Sep 27 '22 12:09 leafac

Coming back here to report success: I changed moduleResolution and I was able to update Got, and for now I enabled skipLibCheck as I work through issues in dependencies. That seems to be what you’re doing too, right, @sindresorhus?

Thank you again for the help!

leafac avatar Oct 02 '22 12:10 leafac