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

Typescript not finding types when using NodeNext #1363

Closed
blahbleepew opened this issue Jan 23, 2023 · 11 comments · Fixed by #1366
Closed

Typescript not finding types when using NodeNext #1363

blahbleepew opened this issue Jan 23, 2023 · 11 comments · Fixed by #1366
Assignees

Comments

@blahbleepew
Copy link

Describe the bug
Typescript fails to find index.d.ts when using NodeNext / esm modules module resolution.

Relevant thread: microsoft/TypeScript#50466

To Reproduce

//tsconfig.json
"compilerOptions": {
    "target": "ESNext",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
}

Here is a patch that fixes it:

diff --git a/package.json b/package.json
index 58aa79f9a78ff9abcbfd82672e89047450e8cb1c..a2fa3818553b8664b03d670ed16c5e160d76c137 100644
--- a/package.json
+++ b/package.json
@@ -31,8 +31,14 @@
   },
   "exports": {
     ".": {
-      "require": "./build/cjs/index.js",
-      "import": "./build/esm/index.js"
+      "require": {
+        "types": "./build/index.d.cts",
+        "default": "./build/cjs/index.cjs"
+      },
+      "import": {
+        "types": "./build/index.d.ts",
+        "default": "./build/esm/index.js"
+      }
     },
     "./compile": {
       "require": "./build/cjs/compile.js",
@thekip
Copy link
Collaborator

thekip commented Jan 24, 2023

What package it's related to?

@dinassh
Copy link

dinassh commented Jan 24, 2023

I have the same issue with the @lingui/core package with versions 3.16.0 and 3.16.1.

The provided patch fixes the issue for me.

@thekip thekip self-assigned this Jan 24, 2023
@thekip
Copy link
Collaborator

thekip commented Jan 24, 2023

What version of typescript was used? I'm trying to reproduce, but everything works seems fine.

You can check my repo and fork if needed
https://github.com/thekip/lingui-nodenext-issue

@thekip
Copy link
Collaborator

thekip commented Jan 24, 2023

Looks like "strict": true is also needed to reproduce

@dinassh
Copy link

dinassh commented Jan 24, 2023

@thekip Thank you for taking a look!

I can reproduce on your example with an additional tsconfig.json compiler option which we are using on our project:

    "strict": true

With this configuration I'm getting an error:

yarn test
yarn run v1.22.19
warning package.json: No license field
$ tsc
index.ts:1:22 - error TS7016: Could not find a declaration file for module '@lingui/core'. '~/lingui-nodenext-issue/node_modules/@lingui/core/build/cjs/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/lingui__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@lingui/core';`

1 import { i18n } from '@lingui/core'
                       ~~~~~~~~~~~~~~


Found 1 error in index.ts:1

@thekip
Copy link
Collaborator

thekip commented Jan 24, 2023

i'm curious should I also generate typings in cjs format (with exports = {} statements), according to the microsoft/TypeScript#50466 i should, but looks like it works even without, but probably would fail in CJS environments, don't know how to check that...

@blahbleepew
Copy link
Author

@thekip I needed to patch @lingui/core and @lingui/react for my project

@thekip
Copy link
Collaborator

thekip commented Jan 25, 2023

Yes, I did it for the both packages

@dinassh
Copy link

dinassh commented Jan 25, 2023

According to the linked discussion it seems separate type declaration files are needed. Although, it seems that the problem is mostly visible when the package has the default exports. I'm not sure at the moment whether the problem is visible with named exports only (like @lingui/core), however it's probably still present.

If I understand correctly, since @lingui/core package.json doesn't contain "type": "module", the ./build/index.d.ts file may be interpreted by TS as a CJS file. If I point both import and require to the same ./build/index.d.ts it seems to work fine with the named exports. However, if I add a default export and try to use it, I get the same error as in the linked thread.

To make TS happy there need to be 2 files .d.cts for CJS and .d.mts for ESM. If there's a .d.ts file it will be interpreted according to the type option in the package.json, so as CJS in this case.

I'm not sure how the current ./build/index.d.ts was generated. If it was generated for ESM, then yes the CJS version may need to be generated. Or vice versa.

@thekip
Copy link
Collaborator

thekip commented Jan 25, 2023

Thanks for the explanation. It seems current solution (with one declaration file) is working. I also have tried to produce declaration with different input settings (module: CommonJs and ESM) and it gave me exactly same typings. So currently I use one typings for both targets and it seems fine.

@dinassh
Copy link

dinassh commented Jan 25, 2023

Thank you taking care of this!

I fear that to make TS perfectly happy there need to be two files even if their content is the same. The issue seems to happen because TS interprets the declaration file type based on package.json type property and on the file extension. So in this case, TS will think the d.ts file is for CJS even if imported to the ESM project.

It seems to work fine in this case when no default exports are present. However, there may be an underlying problem which is currently not visible.

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

Successfully merging a pull request may close this issue.

3 participants