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
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions Sources/Defaults/Defaults.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public enum Defaults {
}

public typealias _Defaults = Defaults

@available(macOS 11, iOS 14, tvOS 14, watchOS 7, *)
Wouter01 marked this conversation as resolved.
Show resolved Hide resolved
public typealias _Default = Default

extension Defaults {
Expand Down
40 changes: 26 additions & 14 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 oldValue != key {
Wouter01 marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -65,14 +75,14 @@ Access stored values from SwiftUI.

This is similar to `@AppStorage` but it accepts a ``Defaults/Key`` and many more types.
*/
@available(macOS 11, iOS 14, tvOS 14, watchOS 7, *)
@propertyWrapper
public struct Default<Value: Defaults.Serializable>: DynamicProperty {
public typealias Publisher = AnyPublisher<Defaults.KeyChange<Value>, Never>

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 +109,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 +132,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 @@ -149,6 +160,7 @@ public struct Default<Value: Defaults.Serializable>: DynamicProperty {
}
}

@available(macOS 11, iOS 14, tvOS 14, watchOS 7, *)
extension Default where Value: Equatable {
/**
Indicates whether the value is the same as the default value.
Expand Down
Loading