search

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

typings for formatNumber #270

Closed audiolion closed 5 years ago

audiolion commented 6 years ago

Hey I notice you have been doing some refactoring, as far as I can see the formatNumber typings seem to be missing the options that can be passed e.g. { fromCountry: CountryCode, humanReadable: boolean }. Am I not understanding something here? Also I used to pass IDD: NumberFormat as the second argument, but the types show it should be CountryCode

catamphetamine commented 6 years ago

Hi. I'm not myself a TypeScript user, the original typings were submitted by contributors. When I'm refactoring something I usually edit the typings but since I don't test them there can be (and often are) some bugs. So, you can modify the typings, test them in your IDE, and then submit a pull request which we then can merge.

audiolion commented 6 years ago

Ok I will look into making a PR back for the types, thanks!

catamphetamine commented 5 years ago

Pushed the updated index.d.ts and custom.d.ts to the github repo. @audiolion You can test them if you want by placing them in node_modules/libphonenumber-js directory. I didn't test them. Will be released at some future patch release I guess.

audiolion commented 5 years ago

That is spooky, I just updated to 1.7.4 and was trying to figure out why some of the types were missing, you hadn't released them yet ;). I will take a look and see, I am not using a ton of pieces of the api, but definitely using the core ones

catamphetamine commented 5 years ago

@audiolion Yeah, I just thought that I'd wait until you check them out to not confuse other people in case there was some syntax error or something.

I am not using a ton of pieces of the api, but definitely using the core ones

Yeah, no need for the full api testing of course. Just see if it works at all and doesn't break everything and that will be enough. You know, could be a missing bracket or something.