-
-
Notifications
You must be signed in to change notification settings - Fork 943
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
fix: type exports for module NodeNext, Node16 #979
Conversation
Codecov Report
@@ 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 |
Just to note, I didn't change the |
You assume? Or you tested it? You can use the playground project to test it: https://github.com/faker-js/playground |
@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 |
There was a problem hiding this 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
?
@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? |
This reverts commit 73db3a7.
When using typescript 4.7rc with module resolution
NodeNext
orNode16
typescript will reportCould not find a declaration file for module @faker-js/faker
. I assume this is because the fallback propertytypes
is pointed set toindex.d.ts
notdist/index.d.ts
and I guess typesVersions is skipped in those module modes.