react-native-unified-contacts icon indicating copy to clipboard operation
react-native-unified-contacts copied to clipboard

phoneNumbers is sometimes undefined

Open jimmythompson opened this issue 6 years ago β€’ 5 comments

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!

jimmythompson avatar Jun 21 '18 08:06 jimmythompson

Thanks for the issue!

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

joshuapinter avatar Jun 21 '18 16:06 joshuapinter

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

jimmythompson avatar Jun 22 '18 13:06 jimmythompson

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.

joshuapinter avatar Jun 22 '18 15:06 joshuapinter

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?

jimmythompson avatar Jun 25 '18 20:06 jimmythompson

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 [email protected], 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.

joshuapinter avatar Jun 25 '18 21:06 joshuapinter