morenoh149 / react-native-contacts

React Native Contacts
MIT License
1.65k stars 564 forks source link

Insufficient Typescript Contact definition #659

Closed mortocks closed 2 years ago

mortocks commented 2 years ago

Please note - my comments here only relate to iOS so it may be that in order to support both iOS and Android, some of my assumptions are incorrect.

Current Typescript Contact type follow this type

interface Contact {
    recordID: string;
    backTitle: string;
    company: string|null;
    emailAddresses: EmailAddress[];
    displayName: string;
    familyName: string;
    givenName: string;
    middleName: string;
    jobTitle: string;
    phoneNumbers: PhoneNumber[];
    hasThumbnail: boolean;
    thumbnailPath: string;
    postalAddresses: PostalAddress[];
    prefix: string;
    suffix: string;
    department: string;
    birthday: Birthday;
    imAddresses: InstantMessageAddress[]
    note: string;
}

In iOS I'm pretty sure all / most of the fields are string | null (NSString | null) properties however only company seems to have this definition, the majority being required strings. The outcome is that you need to provide a lot of empty string properties when creating records which seems incorrect.

Also, iOS supports additional fields which are not implemented in the type definition, mainly

Perhaps the type definitions could support {key: string} definitions so that developers can implement additional fields as needed.

Alternatively (and probably the better solution) the definition could use some kind of typescript generics so that the definition can be extended? e.g

// pseudo code
export function addContact<T extends Contact>(contact: T): Promise<T>;

Also Birthday should probably not be required as a) records don't necessarily have them and b) currently the definition means that you HAVE to supply a fake birthday to create a record

interface Contact {
   ...
   birthday?: Birthday;
}
morenoh149 commented 2 years ago

prs welcome

github-actions[bot] commented 2 years ago

This issue is stale, please provide more information about the status