Skip to content

fix(locale): remove stray whitespace from five locale entries - #4006

Merged
ST-DDT merged 3 commits into
faker-js:nextfrom
MahinAnowar:fix/locale-data-stray-whitespace
Aug 24, 2026
Merged

fix(locale): remove stray whitespace from five locale entries#4006
ST-DDT merged 3 commits into
faker-js:nextfrom
MahinAnowar:fix/locale-data-stray-whitespace

Conversation

@MahinAnowar

Copy link
Copy Markdown
Contributor

Five locale entries carry a leading or trailing space, so faker hands the space back to the caller:

locale definition entry of
ar location.state " بابل" 110
bn_BD location.state "রংপুর " 8
cy book.format "E-lyfr " 4
hu location.city_name "Siófok " 3151
fa lorem.word " و زمان" 89

Each is the only untrimmed entry in its array, and none of the five are affixes, so the space isn't doing anything. faker.location.state() on ar can return " بابل", and hu city names feed location.city(), where the trailing space ends up in the middle of a composed string.

I found these by walking allLocales, so I added the check to locale-data.spec.ts rather than leaving it as five one-off edits. It follows the ignoredCharacterData pattern already in that file, because three definitions legitimately hold whitespace:

const ignoredWhitespaceData = new Set([
  '.location.building_number',
  '.location.city_suffix',
  '.location.street_suffix',
]);

Those are concatenated onto another value, so " aan de IJssel" in nl and " s/n." in es are data, not mistakes. I left them alone. nl.location.city_suffix also contains an empty string, which reads as "sometimes no suffix" and looks deliberate, so I didn't touch that either.

The five locales fail the new check on next and every other locale passes, which is what makes me reasonably confident the ignore list is right rather than just tuned until green. locale-data.spec.ts is green at 2763, and location, book, lorem plus all-functional come to 40045 with no snapshot changes. oxlint and oxfmt are clean.

I can't read Arabic, Bengali, Welsh, Hungarian or Persian, so I've only removed whitespace and changed nothing else about any of these words. Worth a native speaker's glance if that's cheap for you.

One entry each in ar state, bn_BD state, cy book format, hu city name and fa lorem word carried a leading or trailing space, so those values came out of faker with the space attached. Added a locale-data check for it, ignoring the affix definitions where surrounding whitespace is deliberate.
@MahinAnowar
MahinAnowar requested a review from a team as a code owner August 19, 2026 12:17
@netlify

netlify Bot commented Aug 19, 2026

Copy link
Copy Markdown

Deploy Preview for fakerjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 42d00e9
🔍 Latest deploy log https://app.netlify.com/projects/fakerjs/deploys/6a8c1816a85c5f0008b5368c
😎 Deploy Preview https://deploy-preview-4006.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.92%. Comparing base (58e79d4) to head (42d00e9).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #4006      +/-   ##
==========================================
- Coverage   98.98%   98.92%   -0.07%     
==========================================
  Files         926      926              
  Lines        3246     3246              
  Branches      569      582      +13     
==========================================
- Hits         3213     3211       -2     
- Misses         29       31       +2     
  Partials        4        4              

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

matthewmayer
matthewmayer previously approved these changes Aug 19, 2026
Shinigami92
Shinigami92 previously approved these changes Aug 19, 2026
@ST-DDT ST-DDT added this to the v11.0 milestone Aug 19, 2026
@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: locale Permutes locale definitions labels Aug 19, 2026
ST-DDT
ST-DDT previously approved these changes Aug 19, 2026
Comment thread test/locale-data.spec.ts
Comment thread test/locale-data.spec.ts Outdated
@MahinAnowar
MahinAnowar dismissed stale reviews from ST-DDT, Shinigami92, and matthewmayer via 2d0d739 August 24, 2026 10:06
@MahinAnowar

Copy link
Copy Markdown
Contributor Author

Took the array-index suggestion and answered the building_number question inline. Heads up that the push dismissed the three approvals, sorry about that — the only change since is the one-line (e, i) in the test helper. Also merged next in since it had moved.

@ST-DDT
ST-DDT added this pull request to the merge queue Aug 24, 2026
Merged via the queue into faker-js:next with commit c15448e Aug 24, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: locale Permutes locale definitions p: 1-normal Nothing urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants