[ADD] refined combine API

[JonnyBeeGod]

Have you read the Contributing Guidelines?

Issue # (no issue yet, can be created if needed)

Describe your changes

This PR removes the need for the .publisher(for:) KVO API call, the NSObject inheritance and the @objc dynamic annotations by making use of the projectedValue property of the property wrappers. See https://www.avanderlee.com/swift/property-wrappers/#projecting-a-value-from-a-property-wrapper for more details on this topic. Please note that I like to use CurrentValueSubject instead of PassthroughSubject as this makes sure our publisher is stateful and immediately publishes the currently saved value on sink. If we would use PassthroughSubject instead, receiveValue would only fire after the value has been updated again.

This is only a draft PR as this makes discussion way easier rather than describing this change in an issue. Also the example view needs more works and was just thrown together quickly to be able to test the combine API without interference of the manually triggered tableView.reloadRows calls after each update of the values.

Also note the updated deployment targets. This would be required in order to be able to use the Combine publishers

Please let me know what you think

[jessesquires]

Thanks @JonnyBeeGod!

Overall this looks good. 💯

My only hesitation is bumping the minimum deployment targets, which will require a major version update (to 3.0), but maybe that's not terrible since clients can just use the 2.0 version indefinitely...

[JonnyBeeGod]

Thanks @JonnyBeeGod!

Overall this looks good. 💯

My only hesitation is bumping the minimum deployment targets, which will require a major version update (to 3.0), but maybe that's not terrible since clients can just use the 2.0 version indefinitely...

That's great to hear! I updated the PR with your comments, the only thing left would be the example project where I am uncertain on how to proceed. The current example on main branch does not use combine, we could either rewrite it or extend my quickly thrown together Combine example (and maybe also showcase SwiftUI integration). What do you think?

[jessesquires]

I updated the PR with your comments

Thank you! 🙌🏼 Everything is looking good to me. 😎

the only thing left would be the example project where I am uncertain on how to proceed. The current example on main branch does not use combine, we could either rewrite it or extend my quickly thrown together Combine example (and maybe also showcase SwiftUI integration). What do you think?

I'll try to checkout your branch and take a look, then decide. Overall, I think this is mostly ready to go, but I've only done a quick review -- I'll take a closer look soon.

I'm a bit busy with the holidays and such -- but I really appreciate this contribution! 😊

[nolanw]

If Foil changes to vend a publisher via projectedValue, is there any way that publisher could be a UserDefaults KVO publisher for the given key? That would let changes that don't flow through the wrapped property (e.g. another wrapped property with the same key, direct UserDefaults usage, or out-of-process changes to defaults in an app container) notify subscribers to the Foil-wrapped property's publisher.

I'm not sure how to construct a correctly-typed KeyPath to give to the KeyValueObservingPublisher. But if there is some approach that could work with an arbitrary string key (custom publisher doing its own KVO?), the resulting publisher would be more useful and would obviate one way it's currently possible to misuse Foil.

Otherwise, the currently suggested @objc dynamic approach seems more useful than vending a Subject because it allows for Objective-C interoperability.

[jessesquires]

If Foil changes to vend a publisher via projectedValue, is there any way that publisher could be a UserDefaults KVO publisher for the given key?

Thanks for the feedback @nolanw! I'm not entirely sure I'm following here. @JonnyBeeGod can you help?

---

Also, I'm hoping I can circle back to this by the end of this week and get this change landed. 😄

[nolanw]

I kinda went off into the weeds there. More concisely:

The current implementation in this PR adds a CurrentValueSubject to the property wrapper. This works great when the decorated property is mutated, but it misses changes that occur to the underlying UserDefaults value by other means. I was wondering aloud if there's some way to obtain a publisher directly from UserDefaults, because that would capture all changes for the key.

It's certainly not worth holding up this PR, just an idea to ponder. Since the projected value is an AnyPublisher, we can always change it later. If I give my idea a try, I'll open a new PR. Sorry for hijacking this one!

[jessesquires]

@nolanw ah, I see what you mean. 💯

Yes, to accommodate the scenario you describe, you would need a centralized mechanism to access all user defaults.

I believe our suggested usage for Foil in the docs resolves this by creating an AppSettings.shared object through which all settings are accessed and modified. See: https://github.com/jessesquires/Foil#using-combine

This PR is essentially just simplifying the existing pattern by allowing you to omit the call to .publisher(for: ).

But, in any case, yes -- feel free to open a new issue to discuss further! 😊

[jessesquires]

Alright @JonnyBeeGod -- I just pushed a few minor changes. This is ready to go 💯

I'll set it to merge automatically and I'll work on releasing 3.0 by next week.

[jessesquires]

Hm. I think the test failures are flakes. I will address them in #39 before tagging the new release.

[jessesquires]

@JonnyBeeGod I've also invited you to be a collaborator on this project. 😄

[JonnyBeeGod]

@JonnyBeeGod I've also invited you to be a collaborator on this project. 😄

Hey thank you very much :) The invitation already expired but I would still very much like to be a contributor.

[jessesquires]

@JonnyBeeGod -- resent!