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 json data #331

Closed LastDragon-ru closed 5 years ago

LastDragon-ru commented 5 years ago

Not sure that this is the best way, maybe will be better create d.ts files automatically while json to js conversion, unfortunately, I'm not sure how to implement this.

catamphetamine commented 5 years ago

This doesn't look good. First, there's code duplication. Second, it's unclear why would you require typings for JSON.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 3947d3dc5d5225e1551657f5a4ddbeba849cdd9b on LastDragon-ru:json-typings into 9ec9103b9029ac77ad0ac09c8cbc60c18da0ef35 on catamphetamine:master.

LastDragon-ru commented 5 years ago

Second, it's unclear why would you require typings for JSON.

I need custom metadata in angular app, without typings is not possible to use it => https://github.com/catamphetamine/libphonenumber-js/issues/239#issuecomment-494474677

This doesn't look good.

yes, maybe will be better move MetadataJson into types

import {MetadataJson} from './types';

export const metadata: MetadataJson;
export default metadata;

but IMHO convertor should generate these files.

catamphetamine commented 5 years ago

I need custom metadata in angular app, without typings is not possible to use it

No need to import the json file: use the /mobile subpackage for that.

LastDragon-ru commented 5 years ago

No need to import the json file: use the /mobile subpackage for that.

but mobile will use metadata.mobile.json.js, right? I need full and I also need direct access to metadata

    public getCountryCallingCode(country: Country | string): string | null {
        const data = this.metadata.countries[this.getCountryCode(country)] || null;
        const code = data ? data[0] : null;

        return code;
    }
catamphetamine commented 5 years ago

mobile will use metadata.mobile.json.js, right?

It will.

and I also need direct access to metadata

That's your issues.

LastDragon-ru commented 5 years ago

That's your issues.

Nope. Without these typings custom metadata cannot be used in ts.

catamphetamine commented 5 years ago

Nope. Without these typings custom metadata cannot be used in ts.

Show how it can't be used and what's the error message.

LastDragon-ru commented 5 years ago

Show how it can't be used and what's the error message.

Error will be the same as https://github.com/catamphetamine/libphonenumber-js/issues/239#issuecomment-494474677

image

Typescript cannot import js without definitions. Of course, it can be solved by hand but much better when you no need additional work to use the library.

LastDragon-ru commented 5 years ago

Typescript cannot import js without definitions.

So MetadataJson (and ExamplesJson) type required in any case. The *.json.d.ts probably will be better to generate while json->js conversion.

catamphetamine commented 5 years ago

Paste the code that results in the error.

LastDragon-ru commented 5 years ago

Paste the code that results in the error.

import { Component } from '@angular/core';
import {parsePhoneNumberFromString} from 'libphonenumber-js/core';
import metadata from 'libphonenumber-js/metadata.min.json.js';

@Component({
  selector: 'my-app',
  templateUrl: './app.component.html',
  styleUrls: [ './app.component.css' ]
})
export class AppComponent  {
  name = 'Angular';

  test() {
    const parsed = parsePhoneNumberFromString('+7 495 441 12 34', metadata);
    const valid  = parsed != null && parsed.isValid();

    return valid;
  }
}

image

Full source angular-1saffi.zip

catamphetamine commented 5 years ago

Why would you import metadata.min.json manually. There's no need, use the /min package.

LastDragon-ru commented 5 years ago

Why would you import metadata.min.json manually.

Because I need some additional data from metadata? Eg country calling code to output it for user.

catamphetamine commented 5 years ago

Use this instead. https://github.com/catamphetamine/libphonenumber-js/blob/9ec9103b9029ac77ad0ac09c8cbc60c18da0ef35/min/index.d.ts#L49

LastDragon-ru commented 5 years ago

Use this instead.

Ok, thanks. Any way to access to country_calling_codes to find a country by one number? ("+1" - US, "+7" - RU, etc)

catamphetamine commented 5 years ago

Any way to access to country_calling_codes to find a country by one number? ("+1" - US, "+7" - RU, etc)

Multiple countries have the same prefixes.

LastDragon-ru commented 5 years ago

Multiple countries have the same prefixes.

I know. But I need same behavior like https://intl-tel-input.com/ (+7 will ru, not kz)

catamphetamine commented 5 years ago

I know. But I need same behavior like https://intl-tel-input.com/ (+7 will ru, not kz)

+7 is both for RU and KZ.

LastDragon-ru commented 5 years ago

+7 is both for RU and KZ.

Check the example please

catamphetamine commented 5 years ago

Check the example please

No, you check.