fix: use libphonenumber getRegionCodeForNumber for NANP area codes (787->PR, 809->DO, ...) - #11
Open
gaoflow wants to merge 1 commit into
Conversation
…ution
parsePhone() derived countryISOCode from a hand-rolled, incomplete prefixes
table via findCountryISO2, ignoring the bundled libphonenumber's
getRegionCodeForNumber(). NANP area codes missing from the table (787, 809,
829, 268, 284, 340, 649, 784, 868, 869, ...) returned countryISOCode
"nothing" even though the same result object carried the correct areaCode
parsed by libphonenumber.
This produced a self-contradictory result, e.g. parsePhone("17871234567")
returned { areaCode: "787", countryISOCode: "nothing" } -- the library
contradicted its own bundled libphonenumber, which resolves 787 to PR.
Overlay 939 resolved to "PR" while the primary 787 resolved to "nothing".
Fix: after parsing the number with the existing default region, prefer the
authoritative region returned by PhoneNumberUtil.getRegionCodeForNumber()
for countryISOCode, falling back to the prefixes table only when
libphonenumber cannot determine a region (e.g. invalid numbers). This keeps
countryISOCode consistent with the areaCode libphonenumber already parses.
Resolves all missing NANP area codes (787->PR, 809/829->DO, 268->AG, 284->VG,
340->VI, 649->TC, 784->VC, 868->TT, 869->KN) and preserves existing behavior
(939->PR, 849->DO, 1202->US, and non-NANP such as +268 Eswatini).
Added test.js (node assert) covering the RED cases, controls, and the
internal-consistency check (areaCode vs countryISOCode).
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
parsePhone()returns a self-contradictory result for many NANP (North American Numbering Plan) numbers. For example:The
areaCode: "787"is correct — Puerto Rico — and it was parsed by the bundled libphonenumber. YetcountryISOCode: "nothing". The library contradicts its own bundled libphonenumber, whosegetRegionCodeForNumber()returns"PR"for the same input.The library is even inconsistent with itself: the overlay
939resolves to"PR"(it is in the prefixes table) while the primary787resolves to"nothing"(it is not). Same country, opposite answers.Root cause
countryISOCodeis derived from a hand-rolled, incompleteprefixestable viafindCountryISO2, which walks prefixes of the input and looks them up in the map. NANP numbers under country code1are listed per-area-code (e.g.1849: "DO",1939: "PR"), but many area codes are missing:17871234567nothingPR(787 Puerto Rico)18091234567nothingDO(809 Dominican Republic)18291234567nothingDO(829 Dominican Republic overlay)12681234567nothingAG(268 Antigua & Barbuda)1284…,1340…,1649…,1784…,1868…,1869…nothingVG,VI,TC,VC,TT,KNAll of these missing area codes are already known to the bundled libphonenumber, which the package uses to parse
areaCodeinto the very same result object.Fix
Prefer the bundled libphonenumber's authoritative region. After parsing the number with the existing default region, override
countryISOCodewithPhoneNumberUtil.getRegionCodeForNumber(r)when it returns a non-null region, falling back to theprefixestable only when libphonenumber cannot determine a region (e.g. invalid numbers). This keepscountryISOCodeconsistent with theareaCodelibphonenumber already parses.var t = i18n.phonenumbers.PhoneNumberUtil.getInstance() , n = findCountryISO2(e) , o = t.getRegionCodeForCountryCode(n) , r = t.parseAndKeepRawInput("+" + e, o) , d = o = null , i = t.getNationalSignificantNumber(r) , N = t.getLengthOfNationalDestinationCode(r); + var regionForNumber = t.getRegionCodeForNumber(r); + null != regionForNumber && (n = regionForNumber);The
prefixestable andfindCountryISO2are retained as the fallback path and to compute the initial default region for parsing, so non-NANP numbers (e.g.+268Eswatini →SZ) and libphonenumber-null cases are unaffected.Tests
Added
test.js(nodeassert; the package previously had no test suite —npm testwas a stub). Covers:1787→PR,1809→DO,1829→DO,1268→AG, plus1284→VG,1340→VI,1649→TC,1784→VC,1868→TT,1869→KN.1939→PR,1849→DO,1202→US.parsePhone("17871234567")hasareaCode === "787"ANDcountryISOCode === "PR"(the original self-contradiction is gone).parsePhone("26822171234").countryISOCode === "SZ"(Eswatini, country calling code +268, unchanged).Scope
Only
index.js(2 lines) and a newtest.js.findCountryISO2, theprefixestable, and the bundled libphonenumber metadata are unchanged. No new dependencies.