Skip to content

fix: ts support for locales#169

Merged
Shinigami92 merged 12 commits into
mainfrom
locales-ts-support
Jan 19, 2022
Merged

fix: ts support for locales#169
Shinigami92 merged 12 commits into
mainfrom
locales-ts-support

Conversation

@Shinigami92

@Shinigami92 Shinigami92 commented Jan 16, 2022

Copy link
Copy Markdown
Member

No description provided.

@Shinigami92 Shinigami92 added the p: 1-normal Nothing urgent label Jan 16, 2022
@Shinigami92 Shinigami92 self-assigned this Jan 16, 2022
@netlify

netlify Bot commented Jan 16, 2022 •
edited
Loading

Copy link
Copy Markdown

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: 7416f68

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61e81aa34efdfa000788afd8

😎 Browse the preview: https://deploy-preview-169--vigilant-wescoff-04e480.netlify.app
@Shinigami92 Shinigami92 linked an issue Jan 16, 2022 that may be closed by this pull request
Provide TypeScript support for locales #167
Closed
@Shinigami92 Shinigami92 changed the title chore: define type for locales fix: ts support for locales Jan 16, 2022
@Shinigami92

Shinigami92 commented Jan 16, 2022

Copy link
Copy Markdown
Member Author

Oh... I found out that there a locale (without s at the end) that exports for each locale one faker instance 👀
But these are surely currently not typed or covered.

  1. We should move them into src for now so we can migrate them also to TS
  2. As already planned, we should think about a better strategy to load locales
@prisis prisis added this to the v6.0.0 milestone Jan 16, 2022
@Shinigami92

Shinigami92 commented Jan 16, 2022

Copy link
Copy Markdown
Member Author

You need to review this PR with the commits one by one, cause there is at least one commit that has 2x1700 file changes and is not reviewable
@Shinigami92 Shinigami92 marked this pull request as ready for review January 16, 2022 15:56
@Shinigami92 Shinigami92 requested review from a team, damienwebdev, ejcheng and prisis January 16, 2022 16:19
@Shinigami92 Shinigami92 mentioned this pull request Jan 18, 2022
Closed
@Shinigami92 Shinigami92 mentioned this pull request Jan 18, 2022
Closed
@Shinigami92

Shinigami92 commented Jan 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

This is now rebased and can be approved

Nevermind, tests are failing 👀
@Shinigami92

Shinigami92 commented Jan 18, 2022

Copy link
Copy Markdown
Member Author

Needed to use curly brace imports within the locale files, cause the default import is now not the Faker constructor but the default faker instance
This affects only internal behavior 🙂
The external behavior will remain the same, especially after #217
@Shinigami92 Shinigami92 mentioned this pull request Jan 19, 2022
Closed
@Shinigami92 Shinigami92 merged commit 61d0296 into main Jan 19, 2022
@Shinigami92 Shinigami92 deleted the locales-ts-support branch January 19, 2022 15:58
pkuczynski pushed a commit to pkuczynski/faker that referenced this pull request Jan 21, 2022
@Shinigami92 @pkuczynski
fix: ts support for locales (faker-js#169)
c75257c
@ST-DDT ST-DDT mentioned this pull request Nov 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: 1-normal Nothing urgent

3 participants

@Shinigami92 @prisis @JessicaSachs