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

feat: 🔥 add i18n firstname locale #282

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

pumano
Copy link
Contributor

@pumano pumano commented Jun 6, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

add support for i18n 'ru' locale for first name

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

* randFirstName({ length: 10 })
*
*/
export function randFirstName<Options extends NameOptions = never>(
Copy link
Member

Choose a reason for hiding this comment

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

We have discussed this approach in #212. The issue is that we don't have a single source of truth. It'll be better to find a way to use the same functions with different data.

Copy link
Contributor Author

@pumano pumano Jun 6, 2022

Choose a reason for hiding this comment

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

@NetanelBasal single source of truth is really best idea here. But a little problems can be here.

In current case firstname generating with or without accents randomly, while russian locale names (and many others) don't have accents, in that case I need to manually set withAccents: false every time by default. I suggest to broke that default random scenario and generate as "without accents" by default, while, in specific cases when someone needed that extravagant names, they set it to withAccents: true manually.

Also I suggest to:

add locale into core:

export interface LocaleData {
  data: any | string[];
}
export interface FakeOptions {
  length?: number;
  locale?: LocaleData;
}

because it's needed for all rand functions, and when someone want to support locale, just extend from FakeOptions (mostly rand functions extend FakeOptions)

If it's OK, I can provide changes today

Copy link
Member

Choose a reason for hiding this comment

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

Can you please show an example of the usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you mentioned in #212

import { randBird } from '@ngneat/falso';
import ruBird from '@ngneat/falso/i18n/ru/bird';

randBird({ locale: ruBird })

but for randFirstName

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's give it a shot.

@NetanelBasal NetanelBasal merged commit 0997d87 into ngneat:main Jun 7, 2022
@pumano pumano deleted the FEATURE/i18n-firstname branch June 7, 2022 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants