feat(currencies): add ISO 4217 currency support (opt-in) - #166
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds full ISO 4217 currency support to the Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds opt-in ISO 4217 currency support to countries-list via a dedicated countries-list/currencies export so the default entry points remain unchanged in size, while providing richer currency metadata (name, symbols, numeric code, decimals) plus lookup helpers and generated artifacts.
Changes:
- Introduces a generated
currenciesdataset and new currency types (ICurrency,ICurrencyData,TCurrencyCode) plusgetCurrency()/getCurrencyByNumeric(). - Adds build/script support to generate typings, SQL, and minimal JSON subsets for currencies (including “in-use” vs “all” outputs).
- Updates distribution exports (JS/TS/PHP/SQL) and documentation to describe the new opt-in subpath.
Reviewed changes
Copilot reviewed 16 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new opt-in currencies subpath and dataset details. |
| packages/test-js/getCurrency.test.ts | Adds tests for currency lookups and dataset integrity. |
| packages/scripts/utils.ts | Adds script helper to compute “currencies in use”. |
| packages/scripts/tasks/minifyJsonData.ts | Emits currencies JSON plus minimal symbol/numeric maps. |
| packages/scripts/tasks/generateTypings.ts | Generates currencies.d.ts subpath typings and currency code unions. |
| packages/scripts/tasks/generateSql.ts | Adds SQL generation for a currencies table and inserts. |
| packages/scripts/package.json | Adds generate:currencies script entry. |
| packages/scripts/generateCurrencies.ts | New generator to build the currencies dataset from SIX + Intl. |
| packages/scripts/data.ts | Exposes currencies + computed currencies-in-use for build tasks. |
| packages/scripts/constants.ts | Adds CURRENCIES constant for script outputs. |
| packages/countries/tsup.config.ts | Builds a separate opt-in src/currencies.ts entry (CJS/ESM only). |
| packages/countries/src/types.ts | Adds currency types and currency-related record types. |
| packages/countries/src/getCurrency.ts | Implements getCurrency and numeric-code lookup. |
| packages/countries/src/data/currencies.ts | Adds generated ISO 4217 currency dataset. |
| packages/countries/src/currencies.ts | New opt-in entry that re-exports currencies + lookup helpers. |
| dist/package.json | Adds ./currencies export mapping and updates metadata/keywords. |
| dist/mjs/currencies.js | Adds built ESM artifact for the currencies subpath. |
| dist/minimal/currencies.symbol.min.json | Adds minimal code→symbol map output. |
| dist/minimal/currencies.numeric.min.json | Adds minimal code→numeric map output. |
| dist/index.php | Adds PHP loaders for currencies (in-use and all). |
| dist/index.d.ts | Adds emitted currency types and TCurrencyCode union. |
| dist/data.sql | Adds currencies table DDL and insert statements. |
| dist/currencies.min.json | Adds minified “in-use” currencies JSON output. |
| dist/currencies.d.ts | Adds emitted typings for the countries-list/currencies subpath. |
| dist/currencies.all.min.json | Adds minified “all currencies” JSON output. |
| dist/cjs/currencies.js | Adds built CJS artifact for the currencies subpath. |
| composer.json | Updates description/keywords to mention ISO 4217 currencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/countries/src/data/currencies.ts`:
- Around line 265-268: The symbolNative property for the Cape Verdean Escudo
(CVE) currency entry contains an invisible character that renders as blank in
the UI, degrading user experience despite passing tests. Replace the invisible
character in the symbolNative field with a visible fallback value such as the
ISO code (CVE) or the symbol property value to ensure proper display when the
native currency glyph is unavailable or empty.
In `@packages/scripts/utils.ts`:
- Around line 81-95: The code at line 94 silently creates empty objects when a
currency code referenced in the data does not exist in the currencies object,
which can corrupt the output file instead of catching data inconsistencies
early. In the forEach loop that builds currenciesInUse, add a validation check
before the Object.assign call to verify that currencies[code] exists. If the
currency code is not found in the currencies object, throw an error with a
descriptive message that includes the missing currency code so that dataset
drift is surfaced immediately rather than producing incomplete objects in the
final currencies.min.json output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8de1a23-e67a-41a7-91a0-d9ca60446604
⛔ Files ignored due to path filters (11)
dist/cjs/currencies.jsis excluded by!**/dist/**dist/currencies.all.min.jsonis excluded by!**/dist/**dist/currencies.d.tsis excluded by!**/dist/**dist/currencies.min.jsonis excluded by!**/dist/**dist/data.sqlis excluded by!**/dist/**dist/index.d.tsis excluded by!**/dist/**dist/index.phpis excluded by!**/dist/**dist/minimal/currencies.numeric.min.jsonis excluded by!**/dist/**dist/minimal/currencies.symbol.min.jsonis excluded by!**/dist/**dist/mjs/currencies.jsis excluded by!**/dist/**dist/package.jsonis excluded by!**/dist/**
📒 Files selected for processing (16)
README.mdcomposer.jsonpackages/countries/src/currencies.tspackages/countries/src/data/currencies.tspackages/countries/src/getCurrency.tspackages/countries/src/types.tspackages/countries/tsup.config.tspackages/scripts/constants.tspackages/scripts/data.tspackages/scripts/generateCurrencies.tspackages/scripts/package.jsonpackages/scripts/tasks/generateSql.tspackages/scripts/tasks/generateTypings.tspackages/scripts/tasks/minifyJsonData.tspackages/scripts/utils.tspackages/test-js/getCurrency.test.ts
dmythro
added a commit
that referenced
this pull request
Jun 14, 2026
CLDR can return a visually-empty narrow symbol (e.g. CVE native is U+200B), which rendered blank in UIs and slipped past the integrity test: - symbolOf() now falls back to the ISO code when the value has no visible (non-whitespace, non-format) code point - the integrity test requires a visible code point instead of length > 0 - getCurrenciesInUse() throws if a country references a currency missing from the table, instead of silently emitting an empty object Addresses review feedback on #166.
dmythro
force-pushed
the
feat/iso-4217-currencies
branch
from
June 14, 2026 14:57
eab0c08 to
aeec6f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/scripts/generateCurrencies.ts`:
- Line 15: Fix the typo in the URL string at line 15 by correcting "currrency"
(with three consecutive r's) to "currency" (with one r). This typo in the SIX
Group data center URL will cause the fetch operation to fail. Simply correct the
spelling in the URL string to restore functionality.
- Around line 28-35: The local ICurrency interface definition in
generateCurrencies.ts duplicates the canonical ICurrency type exported from
packages/countries/src/types.ts, creating risk of type drift. Remove the local
ICurrency interface definition and instead import ICurrency directly from
packages/countries/src/types.ts at the top of the generateCurrencies.ts file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 465e42f3-48ff-40db-9433-e0ea9546b85c
⛔ Files ignored due to path filters (2)
dist/index.phpis excluded by!**/dist/**dist/package.jsonis excluded by!**/dist/**
📒 Files selected for processing (17)
AGENTS.mdREADME.mdcomposer.jsonpackages/countries/src/currencies.tspackages/countries/src/data/currencies.tspackages/countries/src/getCurrency.tspackages/countries/src/types.tspackages/countries/tsup.config.tspackages/scripts/constants.tspackages/scripts/data.tspackages/scripts/generateCurrencies.tspackages/scripts/package.jsonpackages/scripts/tasks/generateSql.tspackages/scripts/tasks/generateTypings.tspackages/scripts/tasks/minifyJsonData.tspackages/scripts/utils.tspackages/test-js/getCurrency.test.ts
✅ Files skipped from review due to trivial changes (4)
- AGENTS.md
- README.md
- packages/scripts/constants.ts
- composer.json
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/scripts/package.json
- packages/scripts/utils.ts
- packages/countries/src/currencies.ts
- packages/countries/tsup.config.ts
- packages/countries/src/data/currencies.ts
- packages/scripts/tasks/generateSql.ts
- packages/scripts/data.ts
- packages/countries/src/getCurrency.ts
- packages/test-js/getCurrency.test.ts
- packages/scripts/tasks/minifyJsonData.ts
- packages/countries/src/types.ts
- packages/scripts/tasks/generateTypings.ts
dmythro
force-pushed
the
feat/iso-4217-currencies
branch
2 times, most recently
from
June 14, 2026 15:26
12139de to
96a4d29
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/scripts/generateCurrencies.ts`:
- Around line 209-220: The Bun.write() call that writes the currencies.ts file
is executed before validating the completeness of the currency data. Move the
Bun.write() statement to after the missing currency codes check, and add a throw
statement that fails the script when missing.length > 0 to prevent incomplete
generated artifacts from being written to disk.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 774840c9-76d2-468c-8502-a554ecca7619
⛔ Files ignored due to path filters (2)
dist/index.phpis excluded by!**/dist/**dist/package.jsonis excluded by!**/dist/**
📒 Files selected for processing (19)
.gitignoreAGENTS.mdREADME.mdcomposer.jsonpackage.jsonpackages/countries/src/currencies.tspackages/countries/src/data/currencies.tspackages/countries/src/getCurrency.tspackages/countries/src/types.tspackages/countries/tsup.config.tspackages/scripts/constants.tspackages/scripts/data.tspackages/scripts/generateCurrencies.tspackages/scripts/package.jsonpackages/scripts/tasks/generateSql.tspackages/scripts/tasks/generateTypings.tspackages/scripts/tasks/minifyJsonData.tspackages/scripts/utils.tspackages/test-js/getCurrency.test.ts
✅ Files skipped from review due to trivial changes (6)
- packages/scripts/constants.ts
- composer.json
- packages/scripts/data.ts
- AGENTS.md
- .gitignore
- README.md
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/scripts/package.json
- packages/countries/src/currencies.ts
- packages/countries/tsup.config.ts
- packages/countries/src/getCurrency.ts
- packages/scripts/utils.ts
- packages/test-js/getCurrency.test.ts
- packages/scripts/tasks/generateTypings.ts
- packages/scripts/tasks/generateSql.ts
- packages/countries/src/types.ts
- package.json
- packages/scripts/tasks/minifyJsonData.ts
dmythro
force-pushed
the
feat/iso-4217-currencies
branch
from
June 14, 2026 15:50
96a4d29 to
167fa53
Compare
Opt-in `countries-list/currencies` subpath with full ISO 4217 data (English + native name, UI symbol, native symbol, numeric code and minor-unit decimals) for 181 codes — the active list plus withdrawn codes still referenced by country data (flagged `withdrawn`). - `currencies` map, `getCurrency()` and `getCurrencyByNumeric()` utils - `ICurrency` / `ICurrencyData` / `TCurrencyCode` types - `ICountry.currency` is now `TCurrencyCode[]` (validated against the table) - reproducible generator (SIX ISO 4217 list + Unicode CLDR), with a visible-symbol fallback so blank CLDR symbols use the ISO code - SQL table, PHP loaders, README, keywords - fix `check-types` so CI actually type-checks the countries package Data is opt-in: the main bundle is unchanged. Generated dist is built at release, not committed here (see AGENTS.md).
dmythro
force-pushed
the
feat/iso-4217-currencies
branch
from
June 14, 2026 16:15
167fa53 to
49e3679
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds full ISO 4217 currency support as an opt-in
countries-list/currenciessubpath, so the default bundle is unaffected.Each entry is
{ name, native, symbol, symbolNative, numeric, decimals }(+ optionalwithdrawn):name/native— English and home-locale names (e.g.Japanese Yen/日本円).symbol/symbolNative— UI glyph and the glyph used in the currency's home locale.numericis stored as a zero-padded 3-digit string ('008','048') because ISO 4217 numeric codes have significant leading zeros a JS number can't preserve.getCurrencyByNumerictherefore acceptsstring | numberand normalises withString(n).padStart(3, '0').Why opt-in / bundle impact
Currency
constmaps don't reliably tree-shake, so a subpath keeps the main entry flat. Verified: 0 currency bytes in thecjs/mjs/iifeindexentries (main bundle unchanged).mjs/currencies.js(opt-in)currencies.all.min.json(181)currencies.min.json(in-use, 160)minimal/currencies.symbol/.numericData (181 codes)
Pblshd 2026-01-01) for the active set, numeric codes and minor units; Unicode CLDR viaIntlfor the English + native name,symbol(narrow) andsymbolNative. ISO 4217 itself defines no symbols or localized names, hence CLDR. The native name for a multi-country currency uses the primary country's locale (e.g.EUR→euro).packages/scripts/generateCurrencies.ts(bun run generate:currencies), re-run on ISO updates.ANG,SLL,USS), flaggedwithdrawn: true.getCurrencyByNumericresolves shared numerics to the active code (e.g.532→XCG, not the withdrawnANG).symbol/symbolNativefall back to the ISO code (e.g.KWD); blank/zero-width CLDR values are treated as missing.Also in this PR
ICurrency/ICurrencyData/TCurrencyCodetypes;getCurrency()+getCurrencyByNumeric()utils.ICountry.currencytightened toTCurrencyCode[](wasstring[]) — country currency codes are now validated against the table at type-check time.country.currency.includes(someDynamicString)would need a cast.check-typesto actually type-check thecountriespackage — it was a no-op (root tsconfig uses projectreferences+files: [], whichtsc --noEmitignores), so CI was catching zero type errors in the main package.currenciestable;currencies.d.tssubpath typings; PHPcurrencies()/currenciesAll()loaders; README; keywords.Country-data modernization → #167
3 country currency codes are stale vs current ISO 4217 (
ANG→XCG,SLL→SLE, dropUSS). That's a country-data change with consumer impact, so it's a separate stacked PR (#167) rather than bundled here.Notes
dist/is not committed in this PR — it's rebuilt at release (seeAGENTS.md).chore(release)); a minor or major is warranted given theICountry.currencytype change.bun run cipasses (lint + real check-types + build + tests).