faker icon indicating copy to clipboard operation
faker copied to clipboard

Typing of gender conflicts with common usage patterns

Open textbook opened this issue 3 years ago • 7 comments
trafficstars

Describe the bug

Although in practice faker.name.firstName et al. can accept any value, their typing limits them to GenderType | undefined, which is currently 'female' | 'male' | 0 | 1 | undefined. The specific problem with this is that you can't use the return value from faker.name.gender(), whether or not you set binary to true, as the argument to the other methods.

Reproduction

This works fine in JavaScript:

faker.name.firstName(faker.name.gender());

but the exact same code in TypeScript gives:

Argument of type 'string' is not assignable to parameter of type 'GenderType | undefined'.ts (2345)

Additional Info

I found out about this via a Stack Overflow question: https://stackoverflow.com/q/71743871/3001761

textbook avatar Apr 04 '22 22:04 textbook

Attention: faker.name.gender() is not designed to be used as an input value for gender-parameter. faker.name.gender() can return any kind of string and also will not return Male or Female for locale !== en!

Instead of using faker.name.firstName(faker.name.gender()); you should use something like faker.name.firstName(faker.random.arrayElement(['female', 'male'] as GenderType[]));

But then you should start think about why using this in the first place 🤔 You could use just faker.name.firstName(); so without any parameter, so you will also get names that are potential non-fe/male. Otherwise it looks like you are forcefully trying to be non-inclusive?

So use functions like firstName only in three ways:

faker.name.firstName(); // Just return me a name
faker.name.firstName('female'); // I specifically target female (e.g. for a shop?)
faker.name.firstName('male'); // I specifically target male (e.g. for a shop?)

What we could do is to provide a helper function like faker.helpers.genderType(): GenderType. But even then my points above applies as a mindset and you can easily just use faker.random.arrayElement(['female', 'male']) yourself.

Shinigami92 avatar Apr 05 '22 09:04 Shinigami92

Otherwise it looks like you are forcefully trying to be non-inclusive?

I think this has nothing to do with being exclusive, this is just how forms in the web work. If the form allows the selection of genders, there is a huge chance it does not support the same set of genders as we do (which might differ between locales anyway). Our gender() method is only useful for free text gender fields.

However, you pointed out correctly that the gender() method is not suitable for our name methods.

What we could do is to provide a helper function like faker.helpers.genderType(): GenderType.

IMO we should provide it, to allow the user to generate consistent data. Please do not consider GenderType as ['female', 'male'] as they currently are, but as a way to generate consistent data.

In your head it should look like this:

const gender = genderType();
const firstName = firstName(gender);
const lastName = lastName(gender);

and not like in your example:

const gender = 'female';
const firstName = firstName('female');
const lastName = lastName('female');

Maybe to remove the current implementation bias from your/everyones head, we could add a generic/any (as in not specifically male or female/just give me a name) value to the enum and add a binary: boolean parameter to the genderType() method. That way the dev explicitly has the choice, whether they want/can deal with non-binary genders and how they do so. Later we can add more values such as non-binary (as in explicitly both).

That being said, I would like to strictly limit the GenderType to as few values as possible, to avoid huge holes in the translations/non-en locales. Also we should have a well defined fallback strategy in case of missing locale files. But both the additional gender types (beyond generic/any) and their fallbacks are out of scope of this issue.

IMO the genderType() method should be in the name module, because

  • It will be easier to search for/find
  • It is currently only useful in combination with names

For consistency we should add a way to map the gender type to a gender locale value.


TLDR:

  • We should have a genderType() in our name module
  • We should add a generic/any value to it to make it clear, that the enum/type is not about (binary-)genders. It is only meant to generate consistent data.

ST-DDT avatar Apr 05 '22 10:04 ST-DDT

Also some good point. I would like to raise the concern that I understand that it could be useful to add in the name module, but doesn't make much sense in a design perspective, because currently you can expect that everything in the name module is based on locales. I'm also not sure if something like faker.name.helpers.genderType(): GenderType would make sense, because this would be something totally new. We should absolutely discuss this in the team so we can also see what the expectations of other team members would be.

I would not expect that a user's html form matches the API of faker and therefore they already need to write their mapper to be compatible to our API. So I would not like to assume that it should be just designed in that way.

I also wont like to expose a usable parameter like generic, maybe it's just the wrong term but maybe I think it is just too complex to be used. So a signature like firstName(gender?: 'female' | 'male' | 'non-binary'): string already suites all cases that come into my mind. So IMO we would have:

faker.locale = 'de'
faker.name.firstName() // 'Gisela' or 'Herbert' or 'Toni'
faker.name.firstName('female') // 'Gisela'
faker.name.firstName('male') // 'Herbert'
faker.name.firstName('non-binary') // 'Toni'

So if we implement a function like getMeARandomGenderType(), it would break the author's assumption of just getting a female or male first name exclusively, and the author would need to fallback to the provided workaround 🤷 I'm personally totally against to implement a function in faker that returns just ['female', 'male'][randomIndex] in the long term perspective.

Shinigami92 avatar Apr 05 '22 11:04 Shinigami92

I just found out that it is possible to execute the following code:

import { Gender } from '@faker-js/faker'

const genders = Object.values(Gender);
const randomGender = faker.random.objectElement(Gender);

faker.name.firstName(genders[0]); // works
faker.name.firstName(randomGender); // works

Shinigami92 avatar Apr 05 '22 12:04 Shinigami92

After consulting with @Shinigami92 , I think we should call the new value any instead of generic. This value is intentionally added to replace undefined and be cleanly passable to/returnable from a method.

type GenderType = 'any' | 'female' | 'male';
genderType(onlyBinary: boolean = false): GenderType;
firstName(gender: GenderType = 'any')

This also allows more explicit calls like firstName('any').

ST-DDT avatar Apr 05 '22 13:04 ST-DDT

We could also provide export const GENDER_TYPES: readonly GenderType[] as a helper for iterating over all possible input parameters.

Shinigami92 avatar Apr 05 '22 13:04 Shinigami92

We had a HUGE internal and long conversation and we may want to switch GenderType to be 'femme' | 'masc' | 'non-gendered' or something similar. This would have the advantage that you do not misinterpret e.g. Female from faker.name.gender() with the input parameter femme.

We are also currently asking in some communities for feedback and so it could take a while until we proceed further with stuff around GenderType

Edit: Reference-Link: https://twitter.com/_jessicasachs/status/1511375258548379662

Shinigami92 avatar Apr 05 '22 21:04 Shinigami92

@Shinigami92 I dont consider this fixed yet.

ST-DDT avatar Aug 19 '22 08:08 ST-DDT

Fixed by #1289

ST-DDT avatar Sep 08 '22 15:09 ST-DDT