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 5.x isn’t picking up types correctly #10

Open
drwpow opened this issue Apr 3, 2023 · 4 comments · May be fixed by #11
Open

TypeScript 5.x isn’t picking up types correctly #10

drwpow opened this issue Apr 3, 2023 · 4 comments · May be fixed by #11

Comments

@drwpow
Copy link

drwpow commented Apr 3, 2023

Bug

Getting the following error with [email protected] and TypeScript 5:

CleanShot 2023-04-03 at 13 38 47

Could not find a declaration file for module 'vitest-fetch-mock'. '/Users/drew/Sites/drwpow/openapi-fetch/node_modules/.pnpm/[email protected][email protected]/node_modules/vitest-fetch-mock/src/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/vitest-fetch-mock` if it exists or add a new declaration (.d.ts) file containing `declare module 'vitest-fetch-mock';`

Solution

I’ve opened #11 that should resolve the issue. But see the question there on what types/test.ts is doing. I’m not sure how that’s affected.

@drwpow drwpow linked a pull request Apr 3, 2023 that will close this issue
@IanVS
Copy link
Owner

IanVS commented Apr 17, 2023

I guess I don't quite understand why types no longer works for you. I don't see any mention in the breaking change release notes that seems related.

Maybe you could try running typescript with the --traceResolution flag to help find out what's happening?

@drwpow
Copy link
Author

drwpow commented Apr 17, 2023

I noticed this package is still on TypeScript 4.6. 4.7 introduced a breaking change in how "types" works so that it must appear in the package imports (docs). Or, because there are other implications of providing imports (such as all non-specified files are no longer importable), the simpler solution is to merely omit types altogether and rely on the shadowing behavior of .d.ts as my PR did. That will be less disruptive to users than providing imports.

@nxho
Copy link

nxho commented Aug 24, 2023

What are your tsconfig.json settings @drwpow? Is your moduleResolution set to Node16 or NodeNext, and does your package.json have "type": "module"?

I'm guessing this has to do with the moduleResolution property in tsconfig.json and not TypeScript 5.0. If you have "moduleResolution": "Node16" in tsconfig.json and "type": "module" in package.json, TypeScript will use ESM resolution rules for import paths. One of those rules is that under ESM, you cannot do directory imports, which might be why "types": "types" doesn't work, but "types": "types/index.d.ts" would.

If moduleResolution is set to Node, you wouldn't see this error because TypeScript will treat all imports as CommonJS, which allows for directory imports.

All that being said, I ran into this same error, and I have "moduleResolution": "NodeNext" in my tsconfig.json. I worked around this issue by manually pointing TypeScript to index.d.ts by adding this to my compilerOptions:

  "baseUrl": ".",
  "paths": {
    "vitest-fetch-mock": ["node_modules/vitest-fetch-mock/types/index.d.ts"]
  }

But I think this should simply be resolved in vitest-fetch-mock's package.json by simply setting "types": "types/index.d.ts".

Docs on this: https://www.typescriptlang.org/docs/handbook/esm-node.html

@drwpow
Copy link
Author

drwpow commented Sep 8, 2023

What are your tsconfig.json settings @drwpow? Is your moduleResolution set to Node16 or NodeNext, and does your package.json have "type": "module"?

"moduleResolution": "NodeNext" (and "module": "NodeNext"), and yes package.json has "type": "module"—I’m using ESM

I'm guessing this has to do with the moduleResolution property in tsconfig.json and not TypeScript 5.0 … But I think this should simply be resolved in vitest-fetch-mock's package.json by simply setting "types": "types/index.d.ts".

It’s actually 4.7 that made the breaking change. And they discourage setting types in the top-level now in favor of exports, which this package doesn’t use. And I’d be wary of setting that because when adding exports then all files within the package can’t be imported unless explicitly declared in that object (or if a ".*": "./*" glob key is added).

…or as an alternative, you can just remove the "types" key and TypeScript will just associate every .js file with the similarly-named .d.ts file as it’s done for a long time, and the 4.6 vs 4.7 breaking change behavior is sidestepped altogether. Which my PR did 🙂

Either approach will work, but the behavior is very easy to recreate—just update TypeScript in this project >= 4.7, and consume in a project >= 4.7. Perhaps ESM is also the triggering factor for that behavior, ESM as the official module format of JavaScript is now table-stakes for packages to support.

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 a pull request may close this issue.

3 participants