Skip to content
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

infra(docs): add script to regenerate jsdoc examples #3352

Draft
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Dec 29, 2024

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent c: infra Changes to our infrastructure or project setup labels Dec 29, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Dec 29, 2024
@ST-DDT ST-DDT self-assigned this Dec 29, 2024
Copy link

netlify bot commented Dec 29, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit ced8d2d
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/6771ba4d3331be000888005b
😎 Deploy Preview https://deploy-preview-3352.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.

Copy link

codecov bot commented Dec 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (39f76f7) to head (ced8d2d).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #3352      +/-   ##
==========================================
- Coverage   99.97%   99.97%   -0.01%     
==========================================
  Files        2811     2811              
  Lines      217023   217023              
  Branches      941      941              
==========================================
- Hits       216972   216970       -2     
- Misses         51       53       +2     
Files with missing lines Coverage Δ
src/faker.ts 100.00% <ø> (ø)
src/modules/airline/index.ts 100.00% <ø> (ø)
src/modules/animal/index.ts 100.00% <ø> (ø)
src/modules/book/index.ts 100.00% <ø> (ø)
src/modules/color/index.ts 100.00% <ø> (ø)
src/modules/commerce/index.ts 97.71% <ø> (ø)
src/modules/company/index.ts 100.00% <ø> (ø)
src/modules/database/index.ts 100.00% <ø> (ø)
src/modules/date/index.ts 100.00% <ø> (ø)
src/modules/finance/index.ts 100.00% <ø> (ø)
... and 18 more

* faker.color.cmyk({ format: 'binary' }) // (8-32 bits) x 4
* faker.color.cmyk() // [ 0.55, 0.72, 0.6, 0.55 ]
* faker.color.cmyk({ format: 'css' }) // 'cmyk(42%, 65%, 44%, 90%)'
* faker.color.cmyk({ format: 'binary' }) // '00111111011110000101000111101100 00111110110000101000111101011100 00111111010010100011110101110001 00111111000001111010111000010100'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what to do here

Comment on lines +193 to +195
* faker.date.betweens({ from: '2020-01-01T00:00:00.000Z', to: '2030-01-01T00:00:00.000Z' }) // [ '2025-06-27T19:34:39.059Z', '2026-01-10T21:28:14.545Z', '2027-02-25T14:04:55.663Z' ]
* faker.date.betweens({ from: '2020-01-01T00:00:00.000Z', to: '2030-01-01T00:00:00.000Z', count: 2 }) // [ '2024-03-27T14:39:48.843Z', '2025-06-13T10:59:54.311Z' ]
* faker.date.betweens({ from: '2020-01-01T00:00:00.000Z', to: '2030-01-01T00:00:00.000Z', count: { min: 2, max: 5 }}) // [ '2023-11-01T17:05:05.418Z', '2024-05-17T12:08:45.549Z', '2028-12-01T15:31:21.089Z', '2029-08-21T06:14:29.540Z' ]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result formatting has changed from multiline to single line.
Not sure about that.

@@ -86,8 +86,7 @@ export class HackerModule extends ModuleBase {
* Generates a random hacker/IT phrase.
*
* @example
* faker.hacker.phrase()
* // 'If we override the card, we can get to the HDD feed through the back-end HDD sensor!'
* faker.hacker.phrase() // 'Use the online SQL firewall, then you can hack the neural monitor!'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline or on newline?

@@ -881,7 +881,7 @@ export class SimpleHelpersModule extends SimpleModuleBase {
* @param array[].value The value to pick.
*
* @example
* faker.helpers.weightedArrayElement([{ weight: 5, value: 'sunny' }, { weight: 4, value: 'rainy' }, { weight: 1, value: 'snowy' }]) // 'sunny', 50% of the time, 'rainy' 40% of the time, 'snowy' 10% of the time
* faker.helpers.weightedArrayElement([{ weight: 5, value: 'sunny' }, { weight: 4, value: 'rainy' }, { weight: 1, value: 'snowy' }]) // 'rainy'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this one

Comment on lines 1004 to 1010
* faker.helpers.enumValue(Color) // 1
*
* enum Direction { North = 'North', South = 'South'}
* faker.helpers.enumValue(Direction) // 'South'
*
*
* enum HttpStatus { Ok = 200, Created = 201, BadRequest = 400, Unauthorized = 401 }
* faker.helpers.enumValue(HttpStatus) // 200 (Ok)
* faker.helpers.enumValue(HttpStatus) // 400
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@@ -242,7 +242,7 @@ export class PersonModule extends ModuleBase {
* @see faker.person.sex(): For generating a binary-gender value in forms.
*
* @example
* faker.person.sexType() // Sex.Female
* faker.person.sexType() // 'male'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

src/modules/string/index.ts Outdated Show resolved Hide resolved
* faker.system.cron({ includeYear: true }) // '* * * * 5 *'
* faker.system.cron({ includeYear: false }) // '* 22 * * *'
* faker.system.cron({ includeNonStandard: false }) // '40 8 14 9 *'
* faker.system.cron({ includeNonStandard: true }) // '59 2 7 * 4'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably stay as @yearly

*
* // Default behavior
* // faker.setDefaultRefDate();
* faker.date.past(); // Changes based on the current date/time
* faker.date.past(); // '2024-03-11T20:42:37.195Z'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the old value?

*
* // Use a static ref date
* faker.setDefaultRefDate(new Date('2020-01-01'));
* faker.date.past(); // Reproducible '2019-07-03T08:27:58.118Z'
* faker.setDefaultRefDate(new Date('2020-01-01')); // undefined
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to handle this

@matthewmayer
Copy link
Contributor

matthewmayer commented Dec 30, 2024

I'm not sure that the default sample should be completely randomly chosen though.

For example, sometimes the examples are hand picked to illustrate a point. Eg when we chose an example for faker.location.language we deliberately didn't pick English as the example.

* faker.airline.recordLocator({ allowVisuallySimilarCharacters: true }) // 'ANZNEI'
* faker.airline.recordLocator({ allowNumerics: true, allowVisuallySimilarCharacters: true }) // '1Z2Z3E'
* faker.airline.recordLocator() // 'PTQPKR'
* faker.airline.recordLocator({ allowNumerics: true }) // 'FWYDTJ'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another example of where a purely random example is not very helpful

Allow numeric is true, but the default example doesn't include a number

* faker.helpers.fromRegExp(/Joh?n/) // 'Jon'
* faker.helpers.fromRegExp(/ABC*DE/) // 'ABDE'
* faker.helpers.fromRegExp(/bee+p/) // 'beeeeeeeep'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or this example. Sure beep is valid but beeeeeeep is more instructive.

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 30, 2024

I'm not sure that the default sample should be completely randomly chosen though.

Should I use a manual revert for those, or hardcode the values (or the expected condition) into the script?

@matthewmayer
Copy link
Contributor

I don't personally think we need a script which blanket generates all examples. We could maybe have a script which adds missing examples or formats existing ones only?

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 30, 2024

I don't personally think we need a script which blanket generates all examples.

I also considered this.

We could maybe have a script which adds missing examples

Are you referring to the actual code or just the values?

or formats existing ones only?

We would loose type information and would have to parse the old output to format it.
Or am I overthinking this?

One of the reasons I chose generating all (on demand) is to make it more obvious if values show the expected value and not the actual value.

Such as

@matthewmayer
Copy link
Contributor

If you want to see what the actual code does, we now have the refresh button for that? So I don't think this PR adds much value and we might easily remove useful examples by accident if we run it unthinkingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

faker.seed examples are not consistent after refresh
2 participants