elbywan / wretch

A tiny wrapper built around fetch with an intuitive syntax. :candy:
MIT License
4.83k stars 98 forks source link

explicitly provide cjs type #247

Closed marklai1998 closed 2 months ago

marklai1998 commented 2 months ago

When importing wretch in a typescript project where target is set to cjs will yield the following error:

TS1479: The current file is a CommonJS module whose imports will produce require calls; however, the referenced file is an ECMAScript module and cannot be imported with require. Consider writing a dynamic import(wretch)' call instead.

the error is cause by hybriding cjs and esm and mark type: "module"

1 of the suggestion I can find so far is to explicitly generate a .d.cts file and update package.json to point to it

https://github.com/microsoft/TypeScript/issues/53045

elbywan commented 2 months ago

Hey @marklai1998, thanks for reporting the issue :wave:

I did indeed reproduce by using ESM modules syntax in plain .ts files when targeting commonjs (nodenext), but not when using the proper CJS syntax with require() calls - which seems more correct since it matches the target.

Is there a reason why you are not using CJS in the first place inside your typescript files?

It appears that it is quite hard to produce both esm and cjs from a single set of .ts files :confused:.

I read several discussions about the issue in the typescript repo, and it seems like they are unwilling to improve the situation and recommend avoiding mixing the two styles.

I'll try to find a way to produce dual cjs/esm output anyway, but it may take some time :hourglass:.

marklai1998 commented 2 months ago

@elbywan hi, I think its nothing to do with syntax, because you're doing .ts, it has to guess whats the style is(ESM or cjs) with your import call, explicitly doing .cts or .mts will have the same effect(same as .cjs and .mjs) And I think no much ppl will write cjs syntax in ts, its controlled by the target option instead

I see the project also does provide both cjs and esm build, but the issue seems to be with type: module, when its tagged, the other build is ignored. One of the way is to remove type: module and explicitly provide .cjs and .mjs build, otherwise I can't think of other approach other than the one here https://github.com/microsoft/TypeScript/issues/53045 with .d.cts

As for the reason, my company has a lot of nestjs project that has incompatible with ESM(require in ESM is not supported) So you'll have a ts project that target is cj I'm sure there are other ppl has similar experience with other lib as well I can also suppress it with libCheck: false, but don't wanna encourage as a company wise practice

marklai1998 commented 2 months ago

Faker.js also facing this issue https://github.com/faker-js/faker/issues/3087

and I see there is a new PR ppl trying to fix it with .d.cts as well, sounds promising

elbywan commented 2 months ago

I think its nothing to do with syntax, because you're doing .ts, it has to guess whats the style is(ESM or cjs) with your import call, explicitly doing .cts or .mts will have the same effect(same as .cjs and .mjs)

It think that has everything to do with it because typescript uses the subpath exports defined in the wretch package.json file that are syntax dependent.

Using require('wretch') (CJS syntax) works in .ts and .cjs files because it follows the export which points to the explicit .cjs bundle. With import wretch from "wretch" the export points to a file having the .js suffix, and since the nearest package.json is marked as a module it will treat it as ESM no matter what.

Producing .d.cts files is not an viable solution because while it could fix typescript compilation it will still cause node.js to treat emitted .js files as ESM - because of the "type": "module". Renaming the files themselves to .cjs won't work because the emitted imports are rewritten with the .js suffix by typescript, so all the imports should also get rewritten. Honestly I don't want to put too many hacks in place.

Anyway the good news is that I think that I found a proper, mostly clean solution that seems to be working fine: c7f00fc8bdd7

I'll release it shortly after testing it thoroughly.