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

en_IND should be an alias for en_IN in v8 #1454

Closed
9 of 10 tasks
matthewmayer opened this issue Oct 15, 2022 · 4 comments · Fixed by #1873
Closed
9 of 10 tasks

en_IND should be an alias for en_IN in v8 #1454

matthewmayer opened this issue Oct 15, 2022 · 4 comments · Fixed by #1873
Assignees
Labels
c: locale Permutes locale definitions s: accepted Accepted feature / Confirmed bug

Comments

@matthewmayer
Copy link
Contributor

Pre-Checks

Describe the bug

#1354 was fixed in #1448 where the en_IND locale was renamed to en_IN

However to avoid an unnecessary breaking change, in v8 en_IND should be kept as an alias for en_IN, emitting a warning if used, and only completely removed in v9

Minimal reproduction code

No response

Additional Context

No response

Environment Info

System:
    OS: macOS 12.6
    CPU: (8) x64 Apple M1
    Memory: 43.42 MB / 8.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 14.20.0 - /usr/local/bin/node
    npm: 8.19.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 106.0.5249.119
    Firefox: 91.0.2
    Safari: 16.0

Which module system do you use?

  • CJS
  • ESM

Used Package Manager

npm

@matthewmayer matthewmayer added c: bug Something isn't working s: pending triage Pending Triage labels Oct 15, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Oct 15, 2022
@Shinigami92 Shinigami92 added s: accepted Accepted feature / Confirmed bug c: locale Permutes locale definitions and removed c: bug Something isn't working s: pending triage Pending Triage labels Oct 15, 2022
@wael-fadlallah
Copy link
Contributor

@matthewmayer Do you have any insight about how to accomplish this?

@Shinigami92
Copy link
Member

Shinigami92 commented Oct 15, 2022

I think we can just add an if->rewrite here:

faker/src/faker.ts

Lines 53 to 60 in 0f9b0c6

set locale(locale: UsableLocale) {
if (!this.locales[locale]) {
throw new FakerError(
`Locale ${locale} is not supported. You might want to add the requested locale first to \`faker.locales\`.`
);
}
this._locale = locale;
}

But to be fully backward compatible, we also need to allow the old locale in the UsableLocale TypeScript type 🤔

@ST-DDT
Copy link
Member

ST-DDT commented Oct 27, 2022

@Shinigami92 We cannot do that there, because the import has changed. And in v8 we will stop changing/addressing locales via their names/keys.

@xDivisionByZerox
Copy link
Member

We could "add" the en_IND back into our system. But instead of actually implementing it could behave like a proxy to en_IN:

  1. Import en_IN locale
  2. call our deprecated function
  3. Export en_IN as en_IND

Since JS files are simply executed on import, the deprecared function will be called whenever we explicitly import en_IND.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: locale Permutes locale definitions s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants