Fetching new contacts doesn't include recently added contact

[annie-elequin]

Hello, in an Electron app, I am finding that when I'm using this library to add a new contact, I don't see that same contact returned in my "read" operation even though I see it in my Apple Contacts App.

Steps:

1. Get the list of contacts using contacts.getAllContacts()
2. Create a new contact using contacts.addNewContact(content)
3. Get the list of contacts again using contacts.getAllContacts()

Expected Result

The new contact would appear in both the Mac Contacts App and also in the list fetched in Step no. 3

Actual Result

The new contact DID appear in Mac Contacts App, but it did not appear in the list fetched in Step no. 3 When I

Using version 1.6.0 of node-mac-contacts When I CMD+R in the electron app, the operation then fetches the newly added contact correctly

[codebytere]

@annie-elequin could you confirm if this happens in 1.5.0 as well?

[annie-elequin]

@codebytere Just checked, it does not happen in 1.5.0!

Another point of data, in 1.6.0 I added the listener - even if the listener didn't do anything, simply having the listener enabled the second "getAllContacts" to return the updated contact list

[codebytere]

@annie-elequin urgh ok - it's probably https://github.com/codebytere/node-mac-contacts/commit/0412ea6d7ab832263370fd5d7f17ed9db9fb0874 then. The event must be finicky - i think I might just add caching as an option instead of forcing it. Or at minimum allowing caching to be disabled. Thanks for checking! I'll try to get a fix out within a few days.

[annie-elequin]

@codebytere no problem, thanks for your responsiveness! :)

[codebytere]

@annie-elequin alright - so it looks like the issue is that contact caching is implicitly dependent on the event listener, since that's requisite to updating it. For now, i'd recommend just enabling the listener (it doesn't add much if any overhead) in order to ensure the caching works correctly. There are two potential paths forward here:

1. Setup the listener behind the scenes, so that it's no longer manually managed, and caching works by default.
   • A drawback of this approach is i'd need to figure out how to remove the observer since the module doesn't export a class and therefore has no constructor or destructor - that might be hard and take a while.
2. Clarify in docs that calling listener.setup() is a prerequisite to cache updating working correctly
3. Disable caching unless the listener is also in use (so the contacts are always updated and not cached by default), and update the docs accordingly.

What do you think you'd prefer? Also cc @KishanBagaria for your thoughts!

[KishanBagaria]

I think not caching in the library is ideal and the simplest. Most consumers of this lib are aware getAllContacts is expensive and should be memoizing already. In our case, we map and store all the returned contacts in another location in memory to have O(1) lookup by email/phone #.

[annie-elequin]

I don't think no. 1 is necessary since it would be difficult for you - honestly after adding the listener everything is working just fine for us, so I think both no. 2 and no. 3 are perfectly valid options!

I personally think no. 2 makes the most sense, as it feels super clear to the developer what is going on and how they can expect the library to work. If you disable caching though, I definitely think the docs should say something like "i recommend implementing your own form of caching since getAllContacts is expensive" so that people know they should do that