Skip to content

fix: type exports for module NodeNext, Node16#979

Merged
Shinigami92 merged 4 commits intofaker-js:mainfrom
andrew-w-ross:main
May 23, 2022
Merged

fix: type exports for module NodeNext, Node16#979
Shinigami92 merged 4 commits intofaker-js:mainfrom
andrew-w-ross:main

Conversation

@andrew-w-ross
Copy link
Copy Markdown
Contributor

@andrew-w-ross andrew-w-ross commented May 21, 2022

When using typescript 4.7rc with module resolution NodeNext or Node16 typescript will report Could not find a declaration file for module @faker-js/faker. I assume this is because the fallback property types is pointed set to index.d.ts not dist/index.d.ts and I guess typesVersions is skipped in those module modes.

@andrew-w-ross andrew-w-ross requested a review from a team as a code owner May 21, 2022 14:18
@ejcheng ejcheng added the p: 1-normal Nothing urgent label May 21, 2022
@ejcheng ejcheng requested a review from a team May 21, 2022 14:27
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2022

Codecov Report

Merging #979 (fd30611) into main (e51a80d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #979   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files        2110     2110           
  Lines      225693   225693           
  Branches      971      971           
=======================================
  Hits       224950   224950           
  Misses        723      723           
  Partials       20       20           

@andrew-w-ross
Copy link
Copy Markdown
Contributor Author

Just to note, I didn't change the types to dist/types/index.d.ts as I wasn't sure if that was intentional.

@Shinigami92
Copy link
Copy Markdown
Member

I assume this is because the fallback property types is pointed set to index.d.ts not dist/index.d.ts

You assume? Or you tested it? You can use the playground project to test it: https://github.com/faker-js/playground

@andrew-w-ross
Copy link
Copy Markdown
Contributor Author

@Shinigami92 Tested locally as it's an issue with my project currently and playground isn't all that helpful for checking resolutions.

Trace resolution without any changes:

tsc --traceResolution | grep faker
======== Module name '@faker-js/faker' was successfully resolved to '/Users/andrewross/Projects/faker/dist/esm/index.mjs' with Package ID '@faker-js/faker/dist/esm/index.mjs@6.3.1'. ========
src/index.ts(1,23): error TS7016: Could not find a declaration file for module '@faker-js/faker'

And with my change:

tsc --traceResolution | grep faker
======== Resolving module '@faker-js/faker' from '/Users/andrewross/Projects/foo/src/index.ts'. ========
Loading module '@faker-js/faker' from 'node_modules' folder, target file type 'TypeScript'.
Scoped package detected, looking in 'faker-js__faker'
Scoped package detected, looking in 'faker-js__faker'
Scoped package detected, looking in 'faker-js__faker'
Found 'package.json' at '/Users/andrewross/Projects/foo/node_modules/@faker-js/faker/package.json'.
File '/users/andrewross/projects/foo/node_modules/@faker-js/faker/dist/types/index.d.ts' exist - use it as a name resolution result.
Resolving real path for '/users/andrewross/projects/foo/node_modules/@faker-js/faker/dist/types/index.d.ts', result '/Users/andrewross/Projects/faker/dist/types/index.d.ts'.
======== Module name '@faker-js/faker' was successfully resolved to '/Users/andrewross/Projects/faker/dist/types/index.d.ts' with Package ID '@faker-js/faker/dist/types/index.d.ts@6.3.1'. ========

Trace resolution only with types set to ./dist/types/index.d.ts surprisingly seems to have no affect.

Copy link
Copy Markdown
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I currently question myself if https://github.com/andrew-w-ross/faker/blob/f26464e4fda548f90238a81f12491c196f6d7ae7/package.json#L26 should be ./dist/types/index.d.ts 🤔
Do we even have a root index.d.ts?

@andrew-w-ross
Copy link
Copy Markdown
Contributor Author

@Shinigami92 You don't and by the looks of it but you're only going to run into it if you someone uses typescript version < 4 and is that something you really care to support?

@Shinigami92 Shinigami92 requested a review from a team May 22, 2022 19:25
@ejcheng ejcheng added the c: bug Something isn't working label May 23, 2022
@Shinigami92 Shinigami92 enabled auto-merge (squash) May 23, 2022 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: bug Something isn't working p: 1-normal Nothing urgent

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants