sindresorhus / Defaults

💾 Swifty and modern UserDefaults
https://swiftpackageindex.com/sindresorhus/Defaults/documentation/defaults
MIT License
1.92k stars 114 forks source link

Question on 9.0 beta iCloud Syncing #172

Open owenzhao opened 2 months ago

owenzhao commented 2 months ago

Currently I use 8.x with .updates to sync with iCloud manually. However, I find that when syncing with iCloud, it is better to use updates with initial: Bool to false. Or iCloud value will be erased easily by initial value that a new device's newly installed app.

So my question is, is the iCloud Syncing can ignore the initial value by default?

sindresorhus commented 2 months ago

@hank121314 I have confirmed that this is a problem. If I have a Bool key with default value of false, run the app on device A, toggle it to true. Then I run the app on device B. Device A will then have the toggle changed to false again. This is the wrong behavior. The initial value should not override the user set value.

sindresorhus commented 2 months ago

I made a quick example app to test the syncing: Example app.zip

hank121314 commented 2 months ago

I guess this issue is because the value from device A(which is true) has not synced to iCloud storage when device B starts. When device B starts, it cannot find the value in iCloud storage, so it will also sync the default value, which is false, to device A. If the value from device A has already synced to iCloud storage, then when device B starts, it should also have the value true(due to iCloud.add, syncWithoutWaiting(key) will be triggered, using remote storage as the data source).

owenzhao commented 2 months ago

I guess this issue is because the value from device A(which is true) has not synced to iCloud storage when device B starts. When device B starts, it cannot find the value in iCloud storage, so it will also sync the default value, which is false, to device A. If the value from device A has already synced to iCloud storage, then when device B starts, it should also have the value true(due to iCloud.add, syncWithoutWaiting(key) will be triggered, using remote storage as the data source).

Not correct.

This issue is because iCloud always accept the new value as the latest value. So you should not send a value that should not be sent.

For device A, though true was sent to iCloud. But when B starts, B send false, which is initial to iCloud, so false is the new latest and be sent to A from iCloud.

The initial value should never be send. Because without sending it, all device have been already the same as being synced. So it is unnecessary to sync them. Apple knows that, that why all .onChange new API with initial to false as default. On the other hand, Defaults use true as default in legacy, so it may need to change or at least have an option.

hank121314 commented 2 months ago

For device A, though true was sent to iCloud. But when B starts, B send false, which is initial to iCloud, so false is the new latest and be sent to A from iCloud.

I understand what you mean, but in the internal Defaults, we use a timestamp to compare which value is more recent between the UserDefaults and iCloud storage. So when Device B starts, it first fetches the remote timestamp from iCloud and compares it with the local one. Since it is a newly installed app, the local timestamp will be empty. If the remote timestamp is greater or the local timestamp does not exist, we will update the value from remote to local instead.

owenzhao commented 2 months ago

For device A, though true was sent to iCloud. But when B starts, B send false, which is initial to iCloud, so false is the new latest and be sent to A from iCloud.

I understand what you mean, but in the internal Defaults, we use a timestamp to compare which value is more recent between the UserDefaults and iCloud storage. So when Device B starts, it first fetches the remote timestamp from iCloud and compares it with the local one. Since it is a newly installed app, the local timestamp will be empty. If the remote timestamp is greater or the local timestamp does not exist, we will update the value from remote to local instead.

Since it is not need to send the initial value to iCloud at all. Why send it at the first place? Without sending it, you even no need to check the timestamp as iCloud can do all things perfect.

Also, as the device B is newly installed, the initial value timestamp will be latest than the iCloud's, so your compare will fail.

hank121314 commented 2 months ago

Also, as the device B is newly installed, the initial value timestamp will be latest than the iCloud's, so your compare will fail.

The local timestamp will be updated only when it receives the remote update from iCloud or we change the value in UserDefaults. Since device B is newly installed, the initial value timestamp will not exist. For more details, please feel free to read the source code :).

The initial value should never be send. Because without sending it, all device have been already the same as being synced.

I agree with this. However, it will require that the Defaults Value conforms to Equatable to check whether the current value is the initial value.

sindresorhus commented 1 month ago

I agree with this. However, it will require that the Defaults Value conforms to Equatable to check whether the current value is the initial value.

Can we not use UserDefaults.standard.dictionaryRepresentation().keys.contains() to check whether the key has been set?

sindresorhus commented 1 month ago

Alternatively, we could locally track when a key is set locally the first time.