Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add @PublishedDefault for ObservableObject #128

Closed
wants to merge 3 commits into from
Closed

Add @PublishedDefault for ObservableObject #128

wants to merge 3 commits into from

Conversation

Lumisilk
Copy link

@Lumisilk Lumisilk commented Feb 9, 2023

Introduction

Hi. Thanks for the awesome Defaults framework!

This PR add a new property wrapper @PublishedDefault, that is used exclusively for ObservableObject.

@PublishedDefault will trigger objectWillChange when changing the value of Defaults via ObservableObject.

You can use it like this

extension Defaults.Keys {
    static let opacity = Key<Double>("opacity", default: 0.5)
}

final class ViewModel: ObservableObject {
    @PublishedDefault(.opacity) var opacity
}

viewModel.opacity = 1.0
viewModel.objectWillChange // => fire Void
viewModel.$opacity // => fire 1.0

Detail

I used a feature called Referencing the enclosing 'self' in a wrapper type that is not in the Swift documentation but is in the Property Wrapper's Proposal. This feature makes the property wrapper can access the object that holding itself.

However, you can access the enclosing object only when that object reads/write the corresponding value, instead of all the time.

Due to this limitation, @PublishedDefault CANNOT observe changes in the value of the underlying UserDefaults, and can only trigger objectWillChange if the value change is made by the ObservableObject that holds it.

Defaults[.opacity] = 0.2

// neither of the following will fire
viewModel.objectWillChange
viewModel.$opacity

(Well, technically you can let @PublsihedDefaults hold a reference to the object when read/write occurs to make the observation work. But that doesn't smell good, so I didn't try it.)

It would be nice if you can also check the comments.

This PR also fix #48

Other helpful reference:

https://www.swiftbysundell.com/articles/accessing-a-swift-property-wrappers-enclosing-instance/
https://www.avanderlee.com/swift/appstorage-explained/#creating-an-alternative-solution-for-reading-and-writing-user-defaults-through-a-property-wrapper

TODO

  • Use AnyObject as generic constraint
    • try to cast it to ObservableObject if possible, instead of constrain it as ObservableObject only.
  • Fatal Error's message
  • Update @Defaults comment and README
  • Use correct indentation

@Lumisilk Lumisilk changed the title Add @PublishedDefault for ObservedObject Add @PublishedDefault for ObservableObject Feb 9, 2023
@sindresorhus
Copy link
Owner

Thanks for working on this 👍

Have you considered integrating this into @Default, like with @AppStorage? That would be preferable.

@sindresorhus
Copy link
Owner

sindresorhus commented Feb 9, 2023

Due to this limitation, @PublishedDefault CANNOT observe changes in the value of the underlying UserDefaults, and can only trigger objectWillChange if the value change is made by the ObservableObject that holds it.

Maybe something like this: https://github.com/mergesort/Boutique/blob/ad47507dc0e1c4cf1693fd08567457d9caf664b7/Sources/Boutique/Stored.swift#L32-L44

Or this: https://github.com/j1mmyto9/photo-editor-luts-swiftui/blob/529c3b86f5b179c1148a5dfdb90358eba8749e8a/colorful-room/Utility/NestedObservableObject.swift#L33-L45

@PublishedDefault(.opacity) var opacity
}
```
*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*/
*/

Incorrect indentation on the whole comment block.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, how (or what tool did you use) to adjust only the comment block's indentation width?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This:

Screenshot 2023-03-09 at 14 30 26

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, on Xcode.

On Xcode /** */ block, when you hit enter key, the new line will start at 5 space position.
Did you adjust it manually every time?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup...

@Lumisilk
Copy link
Author

Lumisilk commented Feb 10, 2023

About @Default

Since @Published's projectedValue should be Publisher<Value, Never> but @Default for SwiftUI is Binding<Value>, I think the two feature cannot be integrated into one @Default.

About observation

I tried this:

public struct PublishedDefault<Value: _DefaultsSerializable> {
    private var cancellable: AnyCancellable?

    public static subscript<Object: ObservableObject>(
        _enclosingInstance instance: Object,
        wrapped _: ReferenceWritableKeyPath<Object, Value>,
        storage storageKeyPath: ReferenceWritableKeyPath<Object, Self>
    ) -> Value where Object.ObjectWillChangePublisher == ObservableObjectPublisher {
        get {
            subscribeDefault(to: instance, storage: storageKeyPath)
            return instance[keyPath: storageKeyPath].value
        }
        set {
            subscribeDefault(to: instance, storage: storageKeyPath)
            instance[keyPath: storageKeyPath].value = newValue
        }
    }

    public static func subscribeDefault<Object: ObservableObject>(
        to instance: Object,
        storage storageKeyPath: ReferenceWritableKeyPath<Object, Self>
    ) where Object.ObjectWillChangePublisher == ObservableObjectPublisher {
        guard instance[keyPath: storageKeyPath].cancellable == nil else {
            return
        }
        // transfer Defaults publisher to objectWillChange
        instance[keyPath: storageKeyPath].cancellable =
        Defaults.publisher(instance[keyPath: storageKeyPath].key, options: [.prior])
            .sink { [weak instance] change in
                guard let instance, change.isPrior else { return }
                instance.objectWillChange.send()
            }
    }

    // Use Defaults.Publisher as projectedValue directly
}

Making @PublishedDefault catch a reference to the object when read/write occurs, observation works.
But ONLY when the read/write occurred at least once.

Like this:

let viewModel = ViewModel()

Defaults[.opacity] = 1

viewModel.$opacity // not fire
viewModel.objectWillChange // not fire

viewModel.opacity = 1

// now observation starts working
viewModel.$opacity // fire
viewModel.objectWillChange // fire

I'm afraid I cannot find any solution about observation…

@sindresorhus
Copy link
Owner

Since @published's projectedValue should be Publisher<Value, Never> but @default for SwiftUI is Binding, I think the two feature cannot be integrated into one @default.

I wonder how @AppStorage is able to do this then. Maybe they're using some internal hooks.

@sindresorhus
Copy link
Owner

sindresorhus commented Feb 11, 2023

Having it be @PublishedDefault is fine.

@sindresorhus
Copy link
Owner

And you sure the subscript is not called during initialisation?

@sindresorhus
Copy link
Owner

What if in the normal init() we call Self.subscribe() to start listening? Where Self.subscribe() is a static method that sets up the listening once.

@Lumisilk
Copy link
Author

Lumisilk commented Feb 13, 2023

I wonder how @AppStorage is able to do this then

I didn't know AppStorage become compatible with ObservableObject from iOS 14.5. (missed the detail link of reply in #48)

And I finally catch up what's the problem here.
It seems that @Published didn't use this enclosing feature. The objectWillChange's get method injects itself to the @published using the runtime introspection.
We cannot inject the code to the default implementation of ObservableObject.objectWillChange, so there doesn't seem to be a perfect solution:

  • @Default can't be compatible with ObservableObject
  • @AppStorage hard to cover Codable
  • @PublishedDefault can't observe the change

Is my understanding right?


And you sure the subscript is not called during initialisation?

The static subscript, Yes, it's not.


What if in the normal init() we call Self.subscribe() to start listening? Where Self.subscribe() is a static method that sets up the listening once.

What's the Self here? ObservableObject?

@sindresorhus
Copy link
Owner

Is my understanding right?

Yes

What's the Self here? ObservableObject?

PublishedDefault

@Lumisilk
Copy link
Author

Sorry I didn't get your point…

What I want is a property wrapper that can trigger the owner ObservableObject's objectWillChange fire when the corresponding default value changed.

And static subscript(_enclosingInstance instance:, wrapped _:, storage storageKeyPath:) is the only place where you can access the owner object from the property wrapper side. init() or Self.someStaticMethod() can't do it.

@sindresorhus
Copy link
Owner

This is ugly, but one workaround may be to tell the user to do this:

final class ViewModel: ObservableObject {
    @PublishedDefault(.opacity) var opacity

    init() {
        opacity = opacity
    }
}

@sindresorhus
Copy link
Owner

Since @published's projectedValue should be Publisher<Value, Never> but @default for SwiftUI is Binding, I think the two feature cannot be integrated into one @default.

Thinking more about this. That is just a requirement for @Published, not for a property wrapper that wants to trigger an ObservableObject.

For example: https://github.com/fatbobman/CloudStorage/blob/164586fc1bd780e181eae64a0b795e5c3e2c60b9/Sources/CloudStorage/CloudStorage.swift#L13

@sindresorhus
Copy link
Owner

With the latest news about @Observable, I don't think the current approach is going to work out. See: #142

@sindresorhus
Copy link
Owner

Closing this. Let's continue the discussion in #142.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make @Default usable in an ObservableObject
2 participants