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.81k stars 216 forks source link

PhoneNumber class export #425

Closed dsbudiac closed 2 years ago

dsbudiac commented 2 years ago

Would it be possible to export the PhoneNumber class itself? I have a wrapper function that parses accepts multiple types to cast as a PhoneNumber:

function parse(input: string | PhoneNumber) {
  if (input instanceof PhoneNumber) return input
  return parsePhoneNumberWithError(input)
}

Except PhoneNumber is not exported on the package level (only the type definition).

catamphetamine commented 2 years ago

Is it not?

https://github.com/catamphetamine/libphonenumber-js/blob/master/index.d.ts

rattkin commented 2 years ago

Is it not?

https://github.com/catamphetamine/libphonenumber-js/blob/master/index.d.ts

you're linking the type definition.

catamphetamine commented 2 years ago

you're linking the type definition.

Yes

dsbudiac commented 2 years ago

You can't use instanceof PhoneNumber for a type definition. You need the actual class for runtime.

catamphetamine commented 2 years ago

Ah, I see. But, you could change the code so that it works without instanceof PhoneNumber:

function parse(input: string | PhoneNumber) {
  if (input instanceof string) return parsePhoneNumberWithError(input)
  return input
}
lordent commented 2 years ago

I have a same problem

catamphetamine commented 2 years ago

There's no "problem"

lordent commented 2 years ago

I have a function that accepts a string as input, at the output I expect either an email or a phone that is parsed into the PhoneNumber object and another Email object, in my case I do NOT receive a string, I would like to know exactly what it is to give a correct error

catamphetamine commented 2 years ago

Not clear. Provide code and examples.

lordent commented 2 years ago

import parsePhoneNumber, { PhoneNumber } from 'libphonenumber-js'
import parseEmail, { Email } from 'email-js'

const getPhoneOrEmailOrNickName = (input: string): PhoneNumber | Email | string => {
    const number = parsePhoneNumber(input)
    if (number && number.isValid()) {
        return number
    }

    const email = parseEmail(input)
    if (email && email.isValid()) {
        return email
    }

    return input // nickname
}

const myAwesomePureFunction = (input: string) => {
    const login = getPhoneOrEmailOrNickName(input)
    if (login instanceof Email) {
        // call send email otp message
    } else if (login instanceof PhoneNumber) {
        // call send phone number otp message
    } else if (typeof login === 'string') {
        // Do something with nickname
    }

    throw 'Something wrong'
}

/*
Yes, I can write the function like this, but it won't work cleanly, i want that front does not fall with an unexpected exception
*/

const myAwesomePureFunction = (input: string) => {
    const login = getPhoneOrEmailOrNickName(input)
    if (login instanceof Email) {
        // call send email otp message
    } else if (typeof login === 'string') {
        // Do something with nickname
    }

    // call send phone number otp message

}
catamphetamine commented 2 years ago

So you're using instanceof PhoneNumber. The answer is simple:

Change return number to:

return {
  number: number.number,
  ...
}
catamphetamine commented 2 years ago

Or:

const { email, number, nickname } = getPhoneOrEmailOrNickName() 

There's no need to go polymorphic in your case.

lordent commented 2 years ago

Yes, I understand your solution, I just don't understand what is the difficulty in adding a class to the export, thanks

catamphetamine commented 2 years ago

You don't need to use that class. It's not for your use.

ArturBaybulatov commented 1 year ago

I'm too making a universal phone parsing utility, that can accept an unknown value. Within that utility I need to check for already created PhoneNumber instance. I can't use PhoneNumber | unknown as a type, because it still evaluates to unknown.

export const parsePhone = (rawValue: unknown) => {
  if (rawValue instanceof PhoneNumber) return rawValue

  const value = convertToString(rawValue)
  if (value === undefined) return

  return parsePhoneNumberFromString(value)
}

@catamphetamine Please, consider exporting the PhoneNumber class or providing a utility to check if a value is the class instance.

catamphetamine commented 1 year ago

Wouldn't it be sufficient to just export the PhoneNumber type instead of the class itself?

import type { PhoneNumber } from 'libphonenumber-js'

The instanceof PhoneNumber construct wouldn't work in that case, but I heard there's a "type guard" feature in TypeScript that could substitute instanceof operator: https://bobbyhadz.com/blog/typescript-instanceof-only-refers-to-type-but-is-being-used-as-value

So seems like there's no need to export the implementation of the PhoneNumber class from this library. And the PhoneNumber type is already exported.

ArturBaybulatov commented 1 year ago

Oh, yes, I've got it working using some duck typing and the TypeScript's type guard feature:

import { PhoneNumber } from 'libphonenumber-js'
import { isObject } from 'src/utils'

const isPhone = (value: unknown): value is PhoneNumber =>
  isObject(value) &&
  'countryCallingCode' in value &&
  'nationalNumber' in value &&
  'number' in value

const phone: unknown = {}

if (isPhone(phone) && phone.isValid()) {
  console.log(phone.formatInternational()) // OK
}

Thanks!