CloudKitSyncMonitor icon indicating copy to clipboard operation
CloudKitSyncMonitor copied to clipboard

Added a publisher that sends on successful sync

Open jordikitto opened this issue 2 years ago • 1 comments

This is useful for updating content when changes occur after sync

jordikitto avatar Sep 15 '21 01:09 jordikitto

Hi Jordi,

I like the (embarrassing) spelling fixes, and the idea of syncSucceeded, but I'm concerned that the name is misleading, the intended usage kinda duplicates the importState variable (since it's also a publisher), and apps (at least using NSPersistentCloudKitContainer) shouldn't be monitoring CloudKit directly for changes.

The name "syncSucceeded" might lead developers to think they can detect that their app has "finished syncing", which is, as Nick from Apple put it "a fallacy", as in a distributed system, an app must always assume it doesn't have all the data - maybe change it to eventSucceeded.

But, I think the intended purpose of the change is to detect new content changes, which could be done by attaching to $importState and checking for .succeeded being sent, e.g., in bad psuedo-code, syncMonitor.$importState.sync({ if $0 == .succeeded { do_a_thing(); } }), which would make syncSucceeded (or eventSucceeded) redundant. Checking for a successful import that way isn't too pretty though, so maybe we could put the non-psuedocode version of that logic as a variable in SyncMonitor instead of sending a separate event.

But, if an app is using NSPersistentCloudKitContainer, doing either of these checks is poor programming - it should ignore CloudKit completely and should react to changes in CoreData, otherwise it's just asking for weird bugs.

ggruen avatar Sep 17 '21 15:09 ggruen