catamphetamine / libphonenumber-js

A simpler (and smaller) rewrite of Google Android's libphonenumber library in javascript
https://catamphetamine.gitlab.io/libphonenumber-js/
MIT License
2.79k stars 216 forks source link

inconsistent parse result when using National vs International number #176

Closed aitboudad closed 6 years ago

aitboudad commented 6 years ago
parse('0569887076', 'MA')
// result: { "country": "MA", "phone": "569887076" }

parse('+212569887076')
// result: {}

The number is not valid but it can be parsed see https://libphonenumber.appspot.com/phonenumberparser?number=%2B212+569887076

catamphetamine commented 6 years ago

Try the latest version and reply here

On Tue, 6 Feb 2018 at 21:58, Abdellatif Ait boudad notifications@github.com wrote:

parse('0569887076', 'MA')// result: { "country": "MA", "phone": "569887076" }parse('+212569887076')// result: {}

The number is not valid but it can be parsed see https://libphonenumber.appspot.com/phonenumberparser?number=%2B212+569887076

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/catamphetamine/libphonenumber-js/issues/176, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdH77byHy73jAgzxyn1m4RWjDAZt8fDks5tSKDNgaJpZM4R7jn5 .

aitboudad commented 6 years ago

I'm using the latest version "^1.0.13"

catamphetamine commented 6 years ago

Oh, ok, didn't recognize you from the previous issue. Well, yeah, the phone number can be parsed and is a possible number but the country is undefined. See Google's demo: the region is empty. What could parse do in this case? It could return countryPhoneCode instead of country, for example. It would return { countryPhoneCode, country, phone }. It can't be done in the current version because it could break someone's code. It could be done in the next major version though. Or a separate function could be created, like parsePossibleNumber(text), because otherwise parse() would be too complicated. On the other hand, how would one choose between them.

catamphetamine commented 6 years ago

Updated my comment. So, maybe it would make sense, still Google only gives countryPhoneCode and nationalNumber in this case and doesn't even format it in E164 format. This library could be less strict and still could format { countryPhoneCode, country, phone } in E164 without checking phone number validity. I'm not sure yet what should be done here.

aitboudad commented 6 years ago

why not just adding countryCode to the result { countryCode, country, phone } ? if region is empty then we leave country empty

catamphetamine commented 6 years ago

Because it seems like a separate scenario from what parse() normally deals with. The scenario is that a developer grabs a phone number in international format somewhere (e.g. crawls a web page): x+7 111-222 33 44z. This text would get parsed as +71112223344. The only result Google's demo gives in this case is { countryPhoneCode: '7', phone: '1112223344' }. What would one do with such an input. The only way of using such an input seems to be formatting this number in E.164 for storing in a database. Google's demo doesn't allow doing that for some reason but this library could allow it being less strict. In this case the only scenario when such advanced functionality would be needed is parsing a possible (and most likely invalid) E.164 phone number from text. A separate function for that would have sufficed: something like parsePossibleNumber(text) returning a E.164 if a phone number is a possible one (but it's still most likely gonna be invalid). Do you need such a function?

catamphetamine commented 6 years ago

On second thought, there are also cases when parsing local numbers for a given country could return ones which are possible but not valid. In this case a special option passed to parse() would be more appropriate. E.g. parse('1112223344', 'RU', { possible: true }) would return { country: 'RU', phone: '1112223344', possible: true }. Perhaps I'll implement it that way.

catamphetamine commented 6 years ago

I experimented with "possible" numbers parsing and the new version is released with an undocumented feature:

Perhaps this can become an official feature some time after.

catamphetamine commented 6 years ago

I released the new version with "possible" numbers feature. See the updated readme for parse().

Available options:

catamphetamine commented 6 years ago

Demo has been updated https://catamphetamine.github.io/libphonenumber-js/

aitboudad commented 6 years ago

Thanks sound good now!