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.1.0 x Typescript 4.7.2 x Node 16.15 transpilation fails with moduleResolution=Node16 #2051

Closed
2 tasks done
ClementValot opened this issue Jun 2, 2022 · 22 comments
Closed
2 tasks done

Comments

@ClementValot
Copy link

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

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

@akowald
Copy link

akowald commented Jun 6, 2022

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

@sindresorhus
Copy link
Owner

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

@ClementValot
Copy link
Author

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?

@Baune8D
Copy link
Contributor

Baune8D commented Aug 10, 2022

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 😄 ❤️

@szmarczak
Copy link
Collaborator

We hear you @Baune8D ❤️ 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 ❤️

@ClementValot
Copy link
Author

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/index.js@2.0.1'. ========

=> 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

@dscalzi
Copy link

dscalzi commented Sep 2, 2022

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

dscalzi added a commit to dscalzi/Nebula that referenced this issue Sep 2, 2022
@sindresorhus
Copy link
Owner

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 (#2132).

@sindresorhus
Copy link
Owner

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

// @szmarczak ^

@Baune8D
Copy link
Contributor

Baune8D commented Sep 6, 2022

I have opened PRs for both form-data-encoder and then-busboy
octet-stream/form-data-encoder#10
octet-stream/then-busboy#34

@octet-stream
Copy link
Contributor

octet-stream commented Sep 6, 2022

I've released then-busboy@5.2.1 and form-data-encoder@2.1.2 with these fixes. Thanks @Baune8D!

@proton-ab
Copy link

Now that #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.

@Maxim-Mazurok
Copy link

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)

@sindresorhus
Copy link
Owner

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.

@dscalzi
Copy link

dscalzi commented Sep 22, 2022

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.

@leafac
Copy link

leafac commented Sep 23, 2022

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!

@sindresorhus
Copy link
Owner

@leafac
Copy link

leafac commented Sep 27, 2022

@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 👏

@sindresorhus
Copy link
Owner

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.

@leafac
Copy link

leafac commented Sep 27, 2022

Sounds good. Thanks for the information 😁

@leafac
Copy link

leafac commented Oct 2, 2022

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!

mikkopiu added a commit to polarsquad/cinode-api that referenced this issue Jan 10, 2023
Fixes error "'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`" with `got`. Details: <sindresorhus/got#2051>

Also enable library checks for TypeScript validation now that typings are improved.

Native ESM support details: <https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#esm-nodejs>

Requires using `.js` extensions for relative imports.

`got` is also now ESM-only -> fix import path.

Something's broken with `jwt-decode`'s exports, so using `.default` is required (ref: <auth0/jwt-decode#103> and <auth0/jwt-decode#130>).
mifi added a commit to transloadit/uppy that referenced this issue Mar 13, 2023
dogancelik added a commit to dogancelik/twdl that referenced this issue Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants