fix: emit cts types

[Shinigami92]

closes #3087

[netlify]

✅ Deploy Preview for fakerjs ready!

Name	Link
🔨 Latest commit	e07f248
🔍 Latest deploy log	https://app.netlify.com/sites/fakerjs/deploys/66e2fc4e3c5b670008063cc4
😎 Deploy Preview	https://deploy-preview-3093.fakerjs.dev
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[Shinigami92]

@faker-js/maintainers how should we handle following?

test/faker.spec.ts(21,40): error TS2307: Cannot find module '..' or its corresponding type declarations.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (18ab2c7) to head (e07f248).
Report is 1 commits behind head on next.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #3093      +/-   ##
==========================================
- Coverage   99.96%   99.96%   -0.01%     
==========================================
  Files        2776     2776              
  Lines      226260   226260              
  Branches      945      942       -3     
==========================================
- Hits       226183   226182       -1     
- Misses         77       78       +1     

see 2 files with indirect coverage changes

[Shinigami92]

The only difference between the old types, is that the .d.ts have .js suffixes and the .d.cts have .cjs suffixes. That's it 🤷
Everything else is just an exact copy, resulting in an additional ~500kb to be downloaded by million of users weekly...

[Shinigami92]

It looks like that with this change the moduleResolution Node/Node10 is not usable anymore and the build fails with:

index.ts:2:34 - error TS2307: Cannot find module '@faker-js/faker/locale/de' or its corresponding type declarations.
  There are types at 'D:/shinigami/OpenSource/faker-playground/node_modules/@faker-js/faker/dist/locale/de.d.ts',
  but this result could not be resolved under your current 'moduleResolution' setting.
  Consider updating to 'node16', 'nodenext', or 'bundler'.

For whatever reason, this only affects /locale/* imports

[ST-DDT]

@faker-js/maintainers how should we handle following?

test/faker.spec.ts(21,40): error TS2307: Cannot find module '..' or its corresponding type declarations.

It's gone, if you build the project first.

[Shinigami92]

@faker-js/maintainers how should we handle following?

test/faker.spec.ts(21,40): error TS2307: Cannot find module '..' or its corresponding type declarations.

It's gone, if you build the project first.

But previously we only build the types and not the whole project. Do we really want now to build the whole project just to check the types? Or do we want to add a @ts-expect-error / @ts-ignore on this line?

[ST-DDT]

But previously we only build the types and not the whole project. Do we really want now to build the whole project just to check the types?

No, we don't have to build it.

Or do we want to add a @ts-expect-error / @ts-ignore on this line?

We can do that, but I would like to understand why it doesn't work anymore.

[ST-DDT]

It looks like that with this change the moduleResolution Node/Node10 is not usable anymore and the build fails with:

index.ts:2:34 - error TS2307: Cannot find module '@faker-js/faker/locale/de' or its corresponding type declarations.
  There are types at 'D:/shinigami/OpenSource/faker-playground/node_modules/@faker-js/faker/dist/locale/de.d.ts',
  but this result could not be resolved under your current 'moduleResolution' setting.
  Consider updating to 'node16', 'nodenext', or 'bundler'.

For whatever reason, this only affects /locale/* imports

If you keep the type stuff, it works:

  "types": "index.d.ts",
  "typesVersions": {
    ">=5.0": {
      "*": [
        "dist/*"
      ]
    }
  },

[ST-DDT]

We have to add tests/playgrounds for all of this, so that it doesn't silently break.

[mshima]

I played with this branch against #3095 and the missing step is to add typesVersions field again.

[Shinigami92]

Should we merge #3097 before we make such bigger changes to package.json?

• infra: remove package.json warnings #3097

[Shinigami92]

Thanks @xDivisionByZerox for the review, I will have a look into this probably this evening

[Shinigami92]

Repeating #3093 (comment)

Oh my god I think I got it now 💡

faker/package.json

Line 86 in 18ab2c7

"*": [

previously we had * here, but now we use .
so previously the * was put in front of everything, but the . is now a relative start from here
and this is why dist/ is now required and fixes the current problem

[Shinigami92]

This PR now runs green against faker-js/playground#7 on my local machine

[notaphplover]

I just tested it against my issue repro repo and worked properly 😃

[ST-DDT]

Team Decision

If all works well, we will merge and release this soon.
Additional testing and reviews are very much appreciated. ❤️

[mshima]

An additional recommendation is to use .d.cts types in root types and typesVersions. They are only used in node10 resolutions and probably will be used in commonjs projects.
But I haven’t tried that against playground.

[ST-DDT]

An additional recommendation is to use .d.cts types in root types and typesVersions. They are only used in node10 resolutions and probably will be used in commonjs projects. But I haven’t tried that against playground.

Thanks for the information. Do you have a source for that recommendation?
I was unable to find it and chatgpt seemed to be confused regarding that question.

[mshima]

Thanks for the information. Do you have a source for that recommendation?
I was unable to find it and chatgpt seemed to be confused regarding that question.

https://www.typescriptlang.org/tsconfig/#moduleResolution

'node10' (previously called 'node') for Node.js versions older than v10, which only support CommonJS require. You probably won’t need to use node10 in modern code.

According to this, node10 will be used in legacy code that only supports CommonJS.
So a CommonJS type is probably better.