IanVS / vitest-fetch-mock

Vitest mock for fetch, forked from jest-fetch-mock
MIT License
59 stars 7 forks source link

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

Closed drwpow closed 2 months ago

drwpow commented 1 year ago

Bug

Getting the following error with vitest-fetch-mock@0.2.2 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/vitest-fetch-mock@0.2.2_vitest@0.29.8/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.

IanVS commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.