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

Fix for freezes in MenuBarExtra and NavigationStack #158

Merged
merged 5 commits into from
Jan 18, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions Sources/Defaults/SwiftUI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,39 @@ extension Defaults {
final class Observable<Value: Serializable>: ObservableObject {
private var cancellable: AnyCancellable?
private var task: Task<Void, Never>?
private let key: Defaults.Key<Value>

let objectWillChange = ObservableObjectPublisher()

var key: Defaults.Key<Value> {
didSet {
if key != oldValue {
observe()
}
}
}

var value: Value {
get { Defaults[key] }
set {
objectWillChange.send()
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see any harm in keeping this? Unless I'm missing something. See: #144 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that is kept, the menubar will still freeze for some reason. Removing it fixes that, and doesn't seem to affect anything

Copy link
Owner

Choose a reason for hiding this comment

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

@Wouter01 Removing objectWillChange.send() broke animations when using withAnimation, so I will have to add it back unless you can think of a better solution.

Reproduction: Fixture.zip

Defaults[key] = newValue
}
}

init(_ key: Key<Value>) {
self.key = key


observe()
}

deinit {
task?.cancel()
}

func observe() {
// We only use this on the latest OSes (as of adding this) since the backdeploy library has a lot of bugs.
if #available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) {
task?.cancel()

// The `@MainActor` is important as the `.send()` method doesn't inherit the `@MainActor` from the class.
self.task = .detached(priority: .userInitiated) { @MainActor [weak self] in
task = .detached(priority: .userInitiated) { @MainActor [weak self, key] in
for await _ in Defaults.updates(key) {
guard let self else {
return
Expand All @@ -34,7 +48,7 @@ extension Defaults {
}
}
} else {
self.cancellable = Defaults.publisher(key, options: [.prior])
cancellable = Defaults.publisher(key, options: [.prior])
.sink { [weak self] change in
guard change.isPrior else {
return
Expand All @@ -47,10 +61,6 @@ extension Defaults {
}
}

deinit {
task?.cancel()
}

/**
Reset the key back to its default value.
*/
Expand All @@ -71,8 +81,7 @@ public struct Default<Value: Defaults.Serializable>: DynamicProperty {

private let key: Defaults.Key<Value>

// Intentionally using `@ObservedObjected` over `@StateObject` so that the key can be dynamically changed.
@ObservedObject private var observable: Defaults.Observable<Value>
@StateObject private var observable: Defaults.Observable<Value>

/**
Get/set a `Defaults` item and also have the view be updated when the value changes. This is similar to `@State`.
Expand All @@ -99,7 +108,7 @@ public struct Default<Value: Defaults.Serializable>: DynamicProperty {
*/
public init(_ key: Defaults.Key<Value>) {
self.key = key
self.observable = .init(key)
self._observable = .init(wrappedValue: .init(key))
}

public var wrappedValue: Value {
Expand All @@ -122,6 +131,7 @@ public struct Default<Value: Defaults.Serializable>: DynamicProperty {
public var publisher: Publisher { Defaults.publisher(key) }

public mutating func update() {
observable.key = key
_observable.update()
}

Expand Down Expand Up @@ -156,7 +166,6 @@ extension Default where Value: Equatable {
public var isDefaultValue: Bool { wrappedValue == defaultValue }
}

@available(macOS 11, iOS 14, tvOS 14, watchOS 7, *)
extension Defaults {
/**
A SwiftUI `Toggle` view that is connected to a ``Defaults/Key`` with a `Bool` value.
Expand Down
Loading