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

Imports in .d.ts file lack .js extension making them fail to typecheck #35

Open
msteen opened this issue Oct 3, 2024 · 7 comments
Open

Comments

@msteen
Copy link

msteen commented Oct 3, 2024

None of the examples pass the TypeScript (5.5.2 in my case) checker due to package.json having set "type": "module" yet referencing imports without extensions. This causes TypeScript to fail to resolve the files, and causes core functions like signal (a re-export from ./signals) to fail. If I patch the .d.ts files to include the .js extension to the imports, TypeScript is able to resolve them again and properly typechecks.

@mihar-22
Copy link
Member

mihar-22 commented Oct 3, 2024

You most likely have some TS configuration that requires it, best to set skipLibCheck: true to avoid issues with libraries. They do their own type checking with their own tsconfigs.

@mihar-22 mihar-22 closed this as completed Oct 3, 2024
@msteen
Copy link
Author

msteen commented Oct 3, 2024

I do have "skipLibCheck": true already set. Your .d.ts are just not conform "type": "module", while your dist/**/*.js files are. I wish I could point to the right documentation, but the documentation on it makes it hard to find a clear answer. All my other 15+ dependencies (a mixture of commonjs and esm) work fine with regards to their typings, some even provide both .d.mts and .d.ts if they support both commonjs and esm modules. In your case, as you only provide a ESM module, the typings should match.

I believe the rule in TypeScript with regards to .d.ts files is that they are resolved as regular TypeScript modules when containing import statements, and that in TypeScript required the .js extension for ESM modules.

@mihar-22
Copy link
Member

mihar-22 commented Oct 3, 2024

I don't have this issue in any of my projects, using it right now. This issue also hasn't been reported by anyone up until now, so there must be something in your config or setup. Happy to have a quick look if your provide me a minimal repro in StackBlitz or wherever you prefer.

@msteen
Copy link
Author

msteen commented Oct 3, 2024

It has to do with my use of "moduleResolution": "NodeNext", which is the recommend setting for Node.js 12 and higher.

It also mentions this when I hover "NodeNext" in my tsconfig:

NodeNext: This is the recommended setting for libraries and Node.js applications

I finally found it needing .js documented:

Node.js’s interoperability rules between ESM and CJS are reflected in type checking.

And:

.ts/.tsx/.js/.jsx/.d.ts files are ES modules if the nearest ancestor package.json file contains "type": "module", otherwise CommonJS modules.

And also:

When package-relative paths are supported, they resolve under the same rules as any other relative path considering the moduleResolution mode and context. For example, in --moduleResolution nodenext, directory modules and extensionless paths are only supported in require calls, not in imports:

Meaning .d.ts are interpreted as ES modules by TypeScript, hence requiring file extensions.

The minimal way to reproduce this issue ("skipLibCheck": true is not required, but just to show it does not affect the issue):

{
  "compilerOptions": {
    "skipLibCheck": true,
    "module": "NodeNext",
    "moduleResolution": "NodeNext"
  }
}

BTW here is another project for which a similar problem has been reported: lingui/js-lingui#1363

@mihar-22 mihar-22 reopened this Oct 3, 2024
@mihar-22
Copy link
Member

mihar-22 commented Oct 3, 2024

Thanks for the details, appreciate it. I'll get it fixed tomorrow!

@mihar-22
Copy link
Member

mihar-22 commented Oct 3, 2024

I've been using module resolution bundler, not sure if NodeNext is less/more common. Actually based on docs this might be more appropriate:

image

@msteen
Copy link
Author

msteen commented Oct 3, 2024

Thanks for the details, appreciate it. I'll get it fixed tomorrow!

Awesome! This seems to be an interesting library to be able to use signals like those of Solid, on the server side as well, without having to patch Solid into believing it should be using the web build on the server, which feels like a hack.

The key difference between bundler and node16 (and nodenext) is that bundler is basically the non-strict variant. They explicitly mention node16 (and nodenext) as the correct value to use for library authors, as this is the most strict form of resolution, meaning if your library produces output that can be resolved using nodenext, it will work in almost all settings, including the more loose bundler. I think bundler is mostly intended only for consumers, e.g. library users.

In that sense, my use of the more strict nodenext resolution highlights the issue with your .d.ts, which should should match ES conventions for imports when configured as "type": "module".

And given what I am building is a library too, I really want to keep with nodenext, while I am now forced to a different module resolution, drop your library as a dependency or patch it.

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

2 participants