Send on projectedValue publisher for any change

[nolanw]

Hello! Previously, only when using the property wrapper's setter would the projected publisher send a new value. This ignored changes from:

• Other instances of the property wrapper.
• Other libraries that persist to UserDefaults.
• Direct callers of UserDefaults.
• Other processes.

Now the projected publisher sends the new value no matter where the change occurs.

(I brought this up in #38 and figured I'd continue discussion in a PR, instead of an issue. I hope that's alright :)

[nolanw]

It occurs to me this is a breaking change when keys have dots, at signs, or other KVO key path things. I can't figure out a way to observe individual key changes on UserDefaults while supporting e.g. dots in keys. And that's probably not uncommon, if people namespace their keys using (or similar to) bundle identifiers.

[jessesquires]

Thanks @nolanw! 🙌🏼

Took a quick look and I think everything looks good, but I'll review more in-depth this week.

For now, it looks like tests are failing because the new files are not added to the targets in the Xcode project. (I'm assuming this is because you opened the Swift Package directly in Xcode instead of the project file.)

[nolanw]

Oops, fixed the missing reference. You're right, I had the package open and not the .xcodeproj :)

After testing UserDefaults KVO some more, I discovered:

• Keys starting with @ do not notify. (@ elsewhere seems fine.) • Keys with a . anywhere do not notify.

And after further research, I could not find any way to observe out-of-process changes to UserDefaults for non-KVO-able keys.

I'm not sure how many existing users of Foil this PR affects (i.e. publishers that will suddenly go quiet once they update to KVO-based observation). And I'm struggling to come up with a sensible compromise (my best idea so far: use KVO when possible and fall back to wrapper-only). So I would understand completely if you reject this PR in order to maintain backwards compatibility.

[jessesquires]

Thanks so much @nolanw! I really appreciate this! 🙌🏼

I really want to merge this change, I think it's great. (And if it's breaking, we can bump to a new major version. No big deal.)

After testing UserDefaults KVO some more, I discovered:

• Keys starting with @ do not notify. (@ elsewhere seems fine.)
• Keys with a . anywhere do not notify.

And after further research, I could not find any way to observe out-of-process changes to UserDefaults for non-KVO-able keys.

The issue with @ doesn't seem like a big deal. I think that's not very common. However, I agree that using a . in the key name is very common.

---

I want to make sure I understand the change in behavior. So:

• Previously, keys with periods (.) will notify
• After this change, keys with periods will not notify
• And this is because of keyPaths (which include periods)

Is that correct?

I wonder if there's a possible workaround...

[nolanw]

That's correct.

On the plus side, UserDefaults doesn't throw an exception either!

[jessesquires]

I asked for help on Twitter. Thread here: https://twitter.com/jesse_squires/status/1573035243895853056

Other folks confirmed the problem and said they just stopped using periods.

Here's one idea, but not sure if this will work:

https://twitter.com/calicoding/status/1573046708061097984

[nolanw]

I think that latter idea is equivalent to what this PR does. I couldn't find a way to make a Swift KeyPath from a string, so we're using the old Objective-C interface.

Holding out hope on that thread though!

[basememara]

@nolanw this is so good, nice work!

Also great find on the @, . character issue in the key, did not know about that.. but makes sense because of the Swift/Obj-C interoperability with key paths. Since this a bug in the Apple Foundation code that is encountered with or without this library, how about we add an assertion for those characters to crash in debug mode in the ObserverTrampoline initializer:

guard key.rangeOfCharacter(from: CharacterSet(charactersIn: "@.")) == nil else {
        assertionFailure("Cannot observe keys with '@' or '.' characters")
        return
}

userDefaults.addObserver(self, forKeyPath: key, context: nil)

If it makes it to release, it will ignore the observation instead and never fire.. but the persistence to storage will still occur.

[jessesquires]

@nolanw

I think that latter idea is equivalent to what this PR does. I couldn't find a way to make a Swift KeyPath from a string, so we're using the old Objective-C interface.

Ahh, gotcha. 👍🏼

[jessesquires]

how about we add an assertion for those characters to crash in debug mode in the ObserverTrampoline initializer:

Great idea @basememara. What do you think @nolanw?

Also -- sorry for the lag on this PR! 😄 I want to reiterate that I do want to merge this.

[nolanw]

Good idea! I'll add the check.

No worries at all on PR time, I wanna get it right :)

[nolanw]

Assertions added! I considered lazily creating the observer, so people who don't use the projected publisher can continue using the property. But I couldn't come up with a clean way to do it.

[jessesquires]

Hey friends! sorry for the delay here! 😄

I'm planning on merging this today!

[jessesquires]

Rebased this branch via the GH web UI

[jessesquires]

The iOS unit test failure looks like a flake. Passed locally for me.

[jessesquires]

thanks again @nolanw ! 🙌🏼

release 4.0 is out: https://github.com/jessesquires/Foil/releases/tag/4.0.0

I've also invited you to be a collaborator with push access. 😄

[nolanw]

My pleasure! Thanks everyone for the ideas and reviews :)