joshuapinter / react-native-unified-contacts

Your best friend when working with the latest and greatest Contacts Framework in iOS 9+ in React Native.
MIT License
158 stars 57 forks source link

phoneNumbers is sometimes undefined #79

Open jimmythompson opened 6 years ago

jimmythompson commented 6 years ago

Hey folks,

When someone has added a contact in iOS (and I assume the same in Android) without adding any phone numbers for them, the return value from the getContacts call is a little weird.

In this case, I expected the getContacts call to return an empty list:

{
  contacts: [{
    phoneNumbers: []
  }]
}

But, at least as of 0a67cdb, it omits the field entirely from the result:

{
  contacts: [{}]
}

We just got burned by this. Is this something you'd be open to changing?

It means we now have to do a sanity check on the field before playing with the phone numbers:

e.g. contact.phoneNumbers && contact.phoneNumbers.map(...)

Thank you!

joshuapinter commented 6 years ago

Thanks for the issue!

So you'd like to see all the keys, even if they are empty, correct?

jimmythompson commented 6 years ago

Yes please, for lists at least. πŸ˜„

joshuapinter commented 6 years ago

I think I agree with that API. Makes sense to have an empty key but at least have the key present. Allows you to loop through that key's Array without having to check for its presence. When it's empty, the loop just doesn't do anything.

Let's add this to the next major release.

jimmythompson commented 6 years ago

Hey @joshuapinter, I'd like to fix this. πŸ˜„

As it's hard to put in automated tests, is there any standard way you check it all works? Maybe run the example app?

joshuapinter commented 6 years ago

Excellent! Thanks in advance for your help!

A proper test suite is on the TODO list but nothing is setup yet.

The best way, as you mentioned, is to run the example app and make sure it’s all working as expected.

Let me know how it goes! On Jun 25, 2018, 2:56 PM -0600, Jimmy Thompson notifications@github.com, wrote:

Hey @joshuapinter, I'd like to fix this. πŸ˜„ As it's hard to put in automated tests, is there any standard way you check it all works? Maybe run the example app? β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.