MenuBarExtra freezes in a state loop when using Defaults

[othyn]

It appears someone published a similar issue to this, but didn't respond when macOS 13 was retail released: #106

The example remains the same from that issue - in fact that issue is still directly happening exactly as described. With the issue Wouter01 was having being nearly identical in setup to mine.

Attempting to use @Default will break the app and lock it into a state loop:

@main
struct AutoClickerApp: App {
    @Default(.menuBarShowIcon) private var menuBarShowIcon

    var body: some Scene {
        MenuBarExtra(isInserted: $menuBarShowIcon,
                     content: { Label("Hi!") },
                     label: { Image(systemName: "cursorarrow.click.2") })
            .menuBarExtraStyle(.menu)
    }
}

However to provide more insights into the issue, here is the exact error message being given:

2023-07-03 00:27:25.238937+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.240590+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.244646+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.245899+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.249397+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.250887+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.255733+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.
2023-07-03 00:27:25.257386+0100 AutoClicker[8113:47383804] [SwiftUI] Publishing changes from within view updates is not allowed, this will cause undefined behavior.

This will hang the app as soon as the main window loads and is pulled into focus, with the app entering a state loop, with the debug console being spammed with the above error message.

With Xcode's debugger pointing at this line of code as the culprit:

https://github.com/sindresorhus/Defaults/blob/main/Sources/Defaults/SwiftUI.swift#L16

With a screenshot for reference within Xcode:

Although knowing Xcode, that probably isn't the source of the issue...

I'm not really sure how to debug this, so I can't really provide any insights beyond surfacing the problem I'm afraid, although I'll try to assist the best I can.

[sindresorhus]

This is a SwiftUI bug. You can reproduce it without Defaults too:

final class AppState: ObservableObject {
	@Published var isInserted = true
}

@main
struct AppMain: App {
	@StateObject private var appState = AppState()

	var body: some Scene {
		MenuBarExtra(isInserted: $appState.isInserted) {
			Text("A")
		} label: {
			Text("A")
		}
	}
}

The workaround is to use .constant():

final class AppState: ObservableObject {
	@Published var isInserted = true
}

@main
struct AppMain: App {
	@StateObject private var appState = AppState()

	var body: some Scene {
		MenuBarExtra(isInserted: .constant(appState.isInserted)) {
			Text("A")
		} label: {
			Text("A")
		}
	}
}

[othyn]

@sindresorhus Thanks for the prompt reply. I have a few follow up questions if you don't mind me asking.

I may be misunderstanding how .constant() works, but doesn't that mean you loose state reactivity to the wrapped value?

Also, how does @AppStorage get around this issue? As using @AppStorage it works as you'd expect without the need for wrapping in .constant()?

@AppStorage("showMenuBarIcon") private var showMenubarIcon = true

var menuBar: some Scene {
    MenuBarExtra("test", systemImage: "plus", isInserted: $showMenubarIcon) {
// ...

I still think its a bug in Defaults as it doesn't behave as you'd expect, or as is documented. Especially for people perhaps migrating from @AppStorage.

[sindresorhus]

I may be misunderstanding how .constant() works, but doesn't that mean you loose state reactivity to the wrapped value?

It means you won't be notified if the user Command-drags the menu bar item out of the menu bar, yes. So it depends on your use-case.

Also, how does @AppStorage get around this issue? As using @AppStorage it works as you'd expect without the need for wrapping in .constant()?

I guess it doesn't use ObservableObject underneath, like Defaults does. For third-party code, there's no way of implementing reactivity without using ObservableObject though.

I still think its a bug in Defaults as it doesn't behave as you'd expect, or as is documented. Especially for people perhaps migrating from @AppStorage.

It is not a bug in Defaults, as I have demonstrated. I do agree it would be nice to work around it, but I'm not sure of any workaround as ObservableObject has to be used.

[sindresorhus]

Using @AppStorage just for this one seems like an ok workaround. You can still define the key in Defaults and just make sure you match the string key when using @AppStorage.

[sindresorhus]

I can keep this issue open for discoverability until Apple fixes the issue.

[othyn]

@sindresorhus Thanks again for the prompt reply, and the knowledge share!

It means you won't be notified if the user Command-drags the menu bar item out of the menu bar, yes. So it depends on your use-case.

Hmmm. I mean my current use case is reacting to the user Default preference on whether they'd like the menu bar status icon to be visible. So something like:

@Default(.menuBarShowIcon) private var menuBarShowIcon

var menuBar: some Scene {
    MenuBarExtra("test", systemImage: "plus", isInserted: $menuBarShowIcon) {
// ...

Which based on what you demonstrated needs to be replaced with the following to work correctly:

@Default(.menuBarShowIcon) private var menuBarShowIcon

var menuBar: some Scene {
    MenuBarExtra("test", systemImage: "plus", isInserted: .constant($menuBarShowIcon)) {
// ...

Using @AppStorage just for this one seems like an ok workaround. You can still define the key in Defaults and just make sure you match the string key when using @AppStorage.

Good idea, thank you for the suggestion. If the above doesn't work as intended, I'll use this approach instead! 🙌

I guess it doesn't use ObservableObject underneath, like Defaults does. For third-party code, there's no way of implementing reactivity without using ObservableObject though.

It is not a bug in Defaults, as I have demonstrated. I do agree it would be nice to work around it, but I'm not sure of any workaround as ObservableObject has to be used.

Ah. I see now, well thats a pain. Thanks for explaining. At the behest of Apple once again!

I can keep this issue open for discoverability until Apple fixes the issue.

Thanks, its a shame Apple doesn't have a public issue tracker for this stuff for this issue to then be linked against. This one may remain open for a while otherwise 😬

[Wouter01]

@sindresorhus, Isn't this issue caused by the use of @ObservedObject in the property wrapper? The comment states that ObservedObject should be used instead of StateObject, which is fair. However, in Apple's documentation, the following is stated:

Don’t specify a default or initial value for the observed object. Use the attribute only for a property that acts as an input for a view, as in the above example.

This is what's happening in this case.
In a local build, I changed @ObservedObject to @StateObject, and I also removed objectWillChange.send() from here, as it's not needed:

var value: Value {
  get { Defaults[key] }
  set {
    // objectWillChange.send() <-- removed
    Defaults[key] = newValue
  }
}

This fixes the issue, and also issue #146.

As for dynamically changing the key of the wrapper, the watched key could be updated in the ObservableObject, instead of in the property wrapper.
Another issue is that StateObject isn't available in SwiftUI v1. In order to keep compatibility, a combination of State and ObservedObject could be used, although I have not tried this.
Does this sound like a good idea, or am I missing something here? If all looks good to you, I can make a PR.

[sindresorhus]

Isn't this issue caused by the use of @ObservedObject in the property wrapper?

The SwiftUI bug is triggered by that, yes, but using @ObservedObject is completely valid in this case.

Don’t specify a default or initial value for the observed object. Use the attribute only for a property that acts as an input for a view, as in the above example.

That is only advisory and meant to prevent situations where your view model keeps getting re-initiated, but that's not a problem in this case.

and I also removed objectWillChange.send() from here, as it's not needed:

It's not strictly needed, but it does make it guaranteed the change is reflected in the current view loop and not the next one, which could improve perceived performance.

As for dynamically changing the key of the wrapper, the watched key could be updated in the ObservableObject, instead of in the property wrapper.

I'm ok with doing that. We can just target the macOS 11 and iOS 14 in Defaults. Pull request welcome.