faker-js / faker

Generate massive amounts of fake data in the browser and node.js
https://fakerjs.dev
Other
11.86k stars 877 forks source link

docs(usage): type definitions aren't strictly necessary #2840

Closed fzn0x closed 3 weeks ago

fzn0x commented 3 weeks ago

type definitions aren't strictly necessary here

netlify[bot] commented 3 weeks ago

Deploy Preview for fakerjs ready!

Built without sensitive environment variables

Name Link
Latest commit 22b401492cd5cca95621153ba7f071aff2139aa5
Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/662675db58f59800088bc238
Deploy Preview https://deploy-preview-2840.fakerjs.dev
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.96%. Comparing base (b754dc6) to head (22b4014).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## next #2840 +/- ## ======================================== Coverage 99.95% 99.96% ======================================== Files 2973 2973 Lines 212613 212613 Branches 943 601 -342 ======================================== + Hits 212510 212529 +19 + Misses 103 84 -19 ``` [see 2 files with indirect coverage changes](https://app.codecov.io/gh/faker-js/faker/pull/2840/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=faker-js)
ST-DDT commented 3 weeks ago

type definitions aren't strictly necessary here

They might not be necessary there, but we have them there to reduce the cognitive load from the reader to guess what the method/field is supposed to do/return.

See also:

What is your idea/intention with removing those (aka Why)?

fzn0x commented 3 weeks ago

type definitions aren't strictly necessary here

They might not be necessary there, but we have them there to reduce the cognitive load from the reader to guess what the method/field is supposed to do/return.

See also:

What is your idea/intention with removing those (aka Why)?

Typings still included without specifying Users. Screenshot 2024-04-22 003654

fzn0x commented 3 weeks ago

Improve the example for the Users type definition, or remove it altogether to allow users to explore the usage independently without encountering errors after choosing ESM or CJS.

ST-DDT commented 3 weeks ago

So the issue here is that the example on the readme uses a type that isnt specified anywhere on that page.

matthewmayer commented 3 weeks ago

Maybe remove "export" too? If it's just a standalone example.

ST-DDT commented 3 weeks ago

Maybe remove "export" too? If it's just a standalone example.

I'm undecided on that. Maybe a little bit on the keep it part because I hope this pushes the users to reuse their faker methods and it slightly catches more attention.