faker icon indicating copy to clipboard operation
faker copied to clipboard

Clearer documentation for fakerBASE

Open matthewmayer opened this issue 1 year ago • 6 comments

Clear and concise description of the problem

I don't see the value in exposing fakerBASE (or having the base key in allFakers) implemented in #1748 .

import {base} from "@faker-js/faker" is sufficient if you want to create a Faker instance based on base. fakerBASE by itself is not very useful, might cause confusion for new users, and will need special cases for things like #1984

Suggested solution

Don't expose fakerBASE

Alternative

Clearer documentation

Additional context

No response

matthewmayer avatar Apr 04 '23 11:04 matthewmayer

Not exposing fakerBASE is not an option from my side

So if no other maintainer is of another opinion, we need to got the alternative route

Shinigami92 avatar Apr 04 '23 11:04 Shinigami92

Not exposing fakerBASE is not an option from my side

Could you explain why it is not an option, please? That adds more weight to your argument.


and will need special cases for things like https://github.com/faker-js/faker/issues/1984

IMO only by the same amount as custom locales would. E.g. use the "Partial"<MetadataDefinitions> instead of the "Full"MetadataDefinitions as the other locales would require.


fakerBASE by itself is not very useful

IMO fakerBASE adds a lightweight thing to import if you don't need localized data. But that could also be covered by a fakerNONE (or a custom) instance, that doesn't have any locales at all. That we should probably discuss adding as well.


might cause confusion for new users

IMO we should generate jsdocs for all our pre-built faker istances and probably the locales. For the base locale this might actually require some custom text, but IMO that is acceptable. (The same might be true for locales that are a variant of another locale)

ST-DDT avatar Apr 04 '23 11:04 ST-DDT

I think some of the confusion is that on the surface, fakerBASE only appears to contain quite obscure data like faker.database.collation and faker.color.space. You are saying that the value in it is not for that, but for completely non-localized methods like if you just want things like faker.number.int() and faker.helpers.arrayElement()?

matthewmayer avatar Apr 04 '23 11:04 matthewmayer

Could you explain why it is not an option, please? That adds more weight to your argument.

I'm sorry, I was more or less a bit stressed.

So here are some reasons:

  1. I don't want/like to maintain extra stuff for the base locale for that (like defining branches in the script folder)
  2. the base locale and the base instance is theoretically not different then the other locales, it's just the base/ground whatever
  3. calling "it's to complex for unexperienced users" is IMO a really bad reason either the docs should be improved (as you suggested in the alternative section) or the "unexperienced users" should ignore it or get experienced with it 🤷

Shinigami92 avatar Apr 04 '23 17:04 Shinigami92

Changed the title to reflect improving the documentation better instead.

One thing I've been thinking is that it's hard looking at the docs to tell which methods are reliant on the localizable definitions (like faker.person.firstName() or faker.internet.email()) and which are not (like faker.number.int() or faker.helpers.arrayElement()). Perhaps there could be a standardized way to indicate this, like "Localizable." at the end of the description, or an icon.

matthewmayer avatar Apr 05 '23 01:04 matthewmayer

Team Decision

  • We want to generate/add descriptions for fakerBASE and the other faker and locale instances.
    • fakerBASE/base (localeBASE) will use a custom text
    • fakerX/localeX will use a generated text from the metadata

ST-DDT avatar Apr 20 '23 17:04 ST-DDT