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

Fix TypeScript types #393

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix TypeScript types #393

wants to merge 1 commit into from

Conversation

joshkel
Copy link

@joshkel joshkel commented May 11, 2024

Are the Types Wrong (https://arethetypeswrong.github.io/?p=%40hapi%2Fhoek%4011.0.4) was reporting the following errors:

  • Used fallback condition - Import resolved to types through a conditional package.json export, but only after failing to resolve through an earlier condition. This behavior is a TypeScript bug. It may misrepresent the runtime behavior of this import and should not be relied upon.
  • Masquerading as CJS - Import resolved to a CommonJS type declaration file, but an ESM JavaScript file.

This could result in TypeScript errors similar to the following, depending on the specific tsconfig.json options used:

error TS7016: Could not find a declaration file for module '@hapi/hoek'. 'node_modules/@hapi/hoek/lib/index.mjs' implicitly has an 'any' type.
There are types at 'node_modules/@hapi/hoek/lib/index.d.ts', but this result could not be resolved when respecting package.json "exports". The '@hapi/hoek' library may need to update its package.json or typings.

To fix it, I simply copied index.d.ts to index.d.mts, so both the CJS and ESM versions of the library have types, as described in Masquerading as CJS.

Fixes #394

Are the Types Wrong (https://arethetypeswrong.github.io/?p=%40hapi%2Fhoek%4011.0.4) was reporting the following errors:

* Used fallback condition - Import resolved to types through a conditional package.json export, but only after failing to resolve through an earlier condition. This behavior is a TypeScript bug. It may misrepresent the runtime behavior of this import and should not be relied upon.
* Masquerading as CJS - Import resolved to a CommonJS type declaration file, but an ESM JavaScript file.

This could result in TypeScript errors similar to the following, depending on the specific tsconfig.json options used:

> error TS7016: Could not find a declaration file for module '@hapi/hoek'. 'node_modules/@hapi/hoek/lib/index.mjs' implicitly has an 'any' type.
> There are types at 'node_modules/@hapi/hoek/lib/index.d.ts', but this result could not be resolved when respecting package.json "exports". The '@hapi/hoek' library may need to update its package.json or typings.
@Marsup
Copy link
Contributor

Marsup commented May 22, 2024

Shouldn't typescript use this line regardless of the actual file it's going to import? https://github.com/hapijs/hoek/blob/master/package.json#L12

@Marsup
Copy link
Contributor

Marsup commented May 22, 2024

Reading the article, wouldn't it work having import and require as separate objects like in the last part?

@Marsup
Copy link
Contributor

Marsup commented May 22, 2024

Well, I couldn't reproduce your issue with the condition you provided, and some typescript fixes were done a long while ago in 11.0.4, I see in your issue you're still using 11.0.2, could you try upgrading?

@joshkel
Copy link
Author

joshkel commented May 22, 2024

Thank you; I failed to realize I was out of date.

11.0.4 fixes the specific compiler error I was encountering, but the "Masquerading as CJS" issue reported by Are the Types Wrong still exists.

To reproduce:

  1. Set up a TypeScript project as ESM (i.e., package.json "type": "module"), with tsconfig.json "module": "NodeNext", "moduleResolution": "NodeNext".
  2. In a .ts file, use a default import of Hoek: import Hoek from "@hapi/hoek";

TypeScript will compile without errors, but Node will throw an error at runtime:

import Hoek from "@hapi/hoek";
       ^^^^
SyntaxError: The requested module '@hapi/hoek' does not provide an export named 'default'

You could fix this by using separate import and require objects, as in the last example of Masquerading as CJS, but the import and require options would still need to refer to separate .d.ts files. (If I understand correctly, this is because TypeScript needs to know whether a given .d.ts file refers to CommonJS or ESM, and it determines that by working backwards from the file extension (.d.ts or .d.cts or .d.mts) instead of remembering how it worked forward from an import or require object.) If the separate .d.mts file is added, then the new types line isn't needed, because TypeScript can resolve .d.ts using its normal rules.

(I'll also note that this isn't a huge issue - you can avoid it via import * from Hoek from "@hapi/hoek". It mainly depends on how much you care about catching errors at compile-time vs. runtime and how much you care about keeping Are the Types Wrong happy - I'm starting to care about Are the Types Wrong for my own work, because I'm relying on it as I try to work through type errors in my projects' dependencies.)

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 this pull request may close these issues.

TypeScript errors on import
2 participants