docs(internet): document emoji type values

[matthewmayer]

Currently there's no way to see the possible emoji type values without looking at the source code

Adds list to docs:

[codecov]

Codecov Report

Merging #1732 (876854f) into next (0af40f7) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             next    #1732   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files        2340     2340           
  Lines      242624   242624           
  Branches     1103     1103           
=======================================
  Hits       241772   241772           
  Misses        831      831           
  Partials       21       21           

Impacted Files	Coverage Δ
src/modules/internet/index.ts	100.00% <100.00%> (ø)

[xDivisionByZerox]

TBH, I don't see the problem here. The documentation clearly states that types is an array of EmojiType. Sure you can't see directly which exact values are allowed, but know that there will be a custom type.

Maybe a better improvement would be to link to EmojiType which would also require for it to have its own documentation.

[matthewmayer]

TBH, I don't see the problem here. The documentation clearly states that types is an array of EmojiType. Sure you can't see directly which exact values are allowed, but know that there will be a custom type.

Maybe a better improvement would be to link to EmojiType which would also require for it to have its own documentation.

I feel that's a typescript way of thinking. If you're using the library in JavaScript it's just a list of strings and it's unnecessary to know that it's a custom type.

[ST-DDT]

I'm currently unsure what the best representation is. Since the type is only used once it feels tedious to hide it behind a link.
Alternatively, we could make the type hoverable and display a tooltip.

[xDivisionByZerox]

I feel that's a typescript way of thinking. If you're using the library in JavaScript it's just a list of strings and it's unnecessary to know that it's a custom type.

It sure is and I'm all for it. If you don't use typescript in 2023, that's on you.
Even if you use JS you still get intellisense for that type if you try to input a string value.
TBH, since the project was originally forked I've always seen it as a TypeScript first library. You can see this in nearly all functio, as they rely on TypeScript's typesetter to prevent errors. We don't handle edge cases. If the user don't stay conform with the TypeScript definition it's their fault.

Alternatively, we could make the type hoverable and display a tooltip.

That's something that sounds super cool to me (but also very work intensive for a feature used that less).

[matthewmayer]

I'm currently unsure what the best representation is. Since the type is only used once it feels tedious to hide it behind a link.

Alternatively, we could make the type hoverable and display a tooltip.

That's a bad experience on mobile. I like having the possible values listed so for example you can quickly copy paste from the docs. If let's say you want all types EXCEPT flags you can copy paste from the list then delete flags.

[matthewmayer]

Also, I'm not always in an IDE when i want to refer to documentation. For example, I might be reviewing someone else's code in a PR on mobile, or just browsing the web-based documentation before I start implementing a feature;

[ST-DDT]

Does EmojiType by itself add any value to the documentaion or should we just replace that with a list of possible values in our documentation?

[matthewmayer]

In faker.person.firstName for example you just list the possible values for sex

[ST-DDT]

I think it's because it is a simple type we have to adjust the code to handle arrays thereof as well.

[Shinigami92]

I think it's because it is a simple type we have to adjust the code to handle arrays thereof as well.

IMO this is a much more valued fix that we should try to address

[matthewmayer]

i totally get where you're coming from in wanting to automate this, but...

Looks like fixing this "properly" requires editing the typeToText in scripts/apidoc/signature.ts - and this is already a pretty complex piece of code which requires understanding Typescript's type system deeply, reflection etc, and being confident you're not breaking something else in the docs generation. The number of people who have the skills and the motivation to fix this are limited. (translation: i had a quick attempt, and it was beyond me :D)

Whereas editing the docstrings is something that even a new contributor can easily do. So i think one should not let "perfect be the enemy of good", and in general the bar to acceptance for docs improvements should be fairly low. As there's only a few places in the code with such enum-type string array options, might just want to keep it manual for now to avoid distracting core contributors from other more important v8 work.

[Shinigami92]

i totally get where you're coming from in wanting to automate this, but...

Looks like fixing this "properly" requires editing the typeToText in scripts/apidoc/signature.ts - and this is already a pretty complex piece of code which requires understanding Typescript's type system deeply, reflection etc, and being confident you're not breaking something else in the docs generation. The number of people who have the skills and the motivation to fix this are limited. (translation: i had a quick attempt, and it was beyond me :D)

Whereas editing the docstrings is something that even a new contributor can easily do. So i think one should not let "perfect be the enemy of good", and in general the bar to acceptance for docs improvements should be fairly low. As there's only a few places in the code with such enum-type string array options, might just want to keep it manual for now to avoid distracting core contributors from other more important v8 work.

I'm very happy that someone external say what I was trying to say for quite some month now...
I for myself am to afraid to touch any of these scripts and test logics, so this reflects how few people are in the potential to fix or update stuff via these scripts 🫣

[ST-DDT]

I for myself am to afraid to touch any of these scripts and test logics, so this reflects how few people are in the potential to fix or update stuff via these scripts 🫣

The (seeded) test logic is easy IMO, so I won't comment further on this.
As for the examples and deprecation test and the docs scripts in general, you have to decide what you prefer:

• Nobody updating the scripts
• Or nobody checking the examples or updating the docs

(I'm also a docs editor in another project and NONE of the developers or users submitted any real PRs to update the documentation since the last two major versions, ~2+ years, despite requesting it multiple times in writing and staff meetings)

[ST-DDT]

I wrote a short script that checks our repo for all occurances where the type name is used instead of a union:

Diff (click to expand)

Please ignore the incomplete transformations.

- "ColorFormat",
+ "'binary' | 'css' | NumberColorFormat",
- "ColorFormat",
+ "'binary' | 'css' | NumberColorFormat",
- "ColorFormat",
+ "'binary' | 'css' | NumberColorFormat",
- "ColorFormat",
+ "'binary' | 'css' | NumberColorFormat",
- "ColorFormat",
+ "'binary' | 'css' | NumberColorFormat",
- "ColorFormat",
+ "'binary' | 'css' | NumberColorFormat",
- "Casing",
+ "'lower' | 'mixed' | 'upper'",
- "'hex' | ColorFormat",
+ "'binary' | 'css' | NumberColorFormat | 'hex'",
- "(parameters: any[]) => RecordKey",
+ "(parameters: any[]) => number | string | symbol",
- "(obj: Record<RecordKey, RecordKey>, key: RecordKey) => -1 | 0",
+ "(obj: Record<number | string | symbol, number | string | symbol>, key: number | string | symbol) => -1 | 0",
- "RecordKey | RecordKey[]",
+ "(number | string | symbol)[] | number | string | symbol",
- "Record<RecordKey, RecordKey>",
+ "Record<number | string | symbol, number | string | symbol>",
- "readonly Object[]",
+ "readonly { ... }[]",
- "readonly EmojiType[]",
+ "readonly ('activity' | 'body' | 'flag' | 'food' | 'nature' | 'object' | 'person' | 'smiley' | 'symbol' | 'travel')[]",
- "readonly HTTPStatusCodeType[]",
+ "readonly ('clientError' | 'informational' | 'redirection' | 'serverError' | 'success')[]",
- "HTTPProtocolType",
+ "'http' | 'https'",
- "readonly LiteralUnion<AlphaChar, string>[] | string",
+ "readonly ('A' | 'B' | 'C' | 'D' | 'E' | 'F' | 'G' | 'H' | 'I' | 'J' | 'K' | 'L' | 'M' | 'N' | 'O' | 'P' | 'Q' | 'R' | 'S' | 'T' | 'U' | 'V' | 'W' | 'X' | 'Y' | 'Z' | 'a' | 'b' | 'c' | 'd' | 'e' | 'f' | 'g' | 'h' | 'i' | 'j' | 'k' | 'l' | 'm' | 'n' | 'o' | 'p' | 'q' | 'r' | 's' | 't' | 'u' | 'v' | 'w' | 'x' | 'y' | 'z' | string)[] | string",
- "Casing",
+ "'lower' | 'mixed' | 'upper'",
- "readonly LiteralUnion<AlphaNumericChar, string>[] | string",
+ "readonly ('0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | 'A' | 'B' | 'C' | 'D' | 'E' | 'F' | 'G' | 'H' | 'I' | 'J' | 'K' | 'L' | 'M' | 'N' | 'O' | 'P' | 'Q' | 'R' | 'S' | 'T' | 'U' | 'V' | 'W' | 'X' | 'Y' | 'Z' | 'a' | 'b' | 'c' | 'd' | 'e' | 'f' | 'g' | 'h' | 'i' | 'j' | 'k' | 'l' | 'm' | 'n' | 'o' | 'p' | 'q' | 'r' | 's' | 't' | 'u' | 'v' | 'w' | 'x' | 'y' | 'z' | string)[] | string",
- "Casing",
+ "'lower' | 'mixed' | 'upper'",
- "readonly LiteralUnion<NumericChar, string>[] | string",
+ "readonly ('0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | string)[] | string",
- "Casing",
+ "'lower' | 'mixed' | 'upper'",
- "readonly LiteralUnion<AlphaChar, string>[] | string",
+ "readonly ('A' | 'B' | 'C' | 'D' | 'E' | 'F' | 'G' | 'H' | 'I' | 'J' | 'K' | 'L' | 'M' | 'N' | 'O' | 'P' | 'Q' | 'R' | 'S' | 'T' | 'U' | 'V' | 'W' | 'X' | 'Y' | 'Z' | 'a' | 'b' | 'c' | 'd' | 'e' | 'f' | 'g' | 'h' | 'i' | 'j' | 'k' | 'l' | 'm' | 'n' | 'o' | 'p' | 'q' | 'r' | 's' | 't' | 'u' | 'v' | 'w' | 'x' | 'y' | 'z' | string)[] | string",
- "Casing",
+ "'lower' | 'mixed' | 'upper'",
- "readonly LiteralUnion<AlphaNumericChar, string>[] | string",
+ "readonly ('0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | 'A' | 'B' | 'C' | 'D' | 'E' | 'F' | 'G' | 'H' | 'I' | 'J' | 'K' | 'L' | 'M' | 'N' | 'O' | 'P' | 'Q' | 'R' | 'S' | 'T' | 'U' | 'V' | 'W' | 'X' | 'Y' | 'Z' | 'a' | 'b' | 'c' | 'd' | 'e' | 'f' | 'g' | 'h' | 'i' | 'j' | 'k' | 'l' | 'm' | 'n' | 'o' | 'p' | 'q' | 'r' | 's' | 't' | 'u' | 'v' | 'w' | 'x' | 'y' | 'z' | string)[] | string",
- "Casing",
+ "'lower' | 'mixed' | 'upper'",
- "readonly LiteralUnion<NumericChar, string>[] | string",
+ "readonly ('0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | string)[] | string",

Diff: docs/unwrap-union-type-arrays

[matthewmayer]

i think you really need a human to review these and see where it makes sense to spell it out. For example, spelling out the EmojiType and HTTPStatusCodeType values is useful, whereas spelling out ('A' | 'B' | 'C' | 'D' | 'E' | 'F' | 'G' | 'H' ... forLiteralUnion<AlphaChar, string> isn't useful.

[ST-DDT]

I will merge this for the later docs additions and temp type listing.
I will remove the manual list, when I unwrap the union types.

[ST-DDT]

Thanks for your contributions! ❤️