ViewStores' state out of sync

[rurza]

TCA version: 0.28.1

So I discovered kind of strange behaviour in my app.
I have a controller that is observing changes of the state to drive app's status bar menu (the app is for Mac ☺️).

viewStore.publisher.settingsState
            .proUnlocked
            .combineLatest(viewStore.publisher.playerState,
                           viewStore.publisher.lastFmState.accountState,
                           viewStore.publisher.persistenceState.playedTracks)
            .sink { [weak self] _ in
                self?.updateMenu()
            }
            .store(in: &cancellables)

the updateMenu was a huge method when I was creating brand new menu (NSMenu) from current state. I decided to refactor the code a little bit and move it to dedicated types.
Some of them also requires the state, but they also require to send TCA's actions, that's why I'm passing the Store (and the status bar controller is the owner of those objects).
The code:

(…)
private lazy var lastFmMenuItemFactory = LastFmMenuItemFactory(store: store)

func updateMenu()  {
        guard let menu = self.statusItem?.menu else { return }
        (…)
        if let lastFmMenuItem = self.lastFmMenuItemFactory.menuItem() {
            menu.addItem(lastFmMenuItem)
            menu.addItem(NSMenuItem.separator())
        }
        (…)
}
(…)

Now the problem is, that this LastFmMenuItemFactory class reads the same state that called the updateMenu() method in the first place, but sometimes it's different (it's nil or contains the old value)

class LastFmMenuItemFactory {

    init(store: Store<AppState, AppAction>) {
        self.store = store
    }

    let store: Store<AppState, AppAction>

    lazy var viewStore: ViewStore<AppState, AppAction>  = ViewStore(store)

    func menuItem() -> NSMenuItem? {
            (…)
            // Sometimes the currentTrack is nil
            if let track = viewStore.state.playerState.currentPlayerState?.currentTrack {
                let loveTrackMenuItem = NSMenuItem(title: NSLocalizedString("Love \"\(track.title)\"",
                                                                            comment: "Love currently playing track"),
                                                   action: #selector(loveCurrentTrack),
                                                   keyEquivalent: "")
                loveTrackMenuItem.target = self
                menu.addItem(loveTrackMenuItem)
            } 
            (…)
     }
}

I fixed it by creating a computed variable for the ViewStore, instead of storing it. Another interesting fact is, that this functionality broke when I changed the implementation of the Effect that actually provides the currentTrack to the State.
Current, async implementation (with removed parts that are irrelevant and added comment):

private let getTrackInfoFromPlayerEffect: (PlayerType, GetTrackInfoFlag) -> Effect<PlayerTrack, PlayerTrackEnvironment.Error> = { playerType, flag in
    return Effect.task(priority: .high) { () -> PlayerTrack in
                let task: Task<PlayerTrack, Error> = Task(priority: .high) {
                    let player = _playerForPlayerType(playerType)
                    // this will execute the AppleScript and fetch the data
                    //  it can take even 400ms
                    if let track = player.currentTrack as? PlayerTrack {
                        if something is wrong {
                            throw PlayerTrackEnvironment.Error(someError)
                        } else {
                            return track
                        }
                    } else {
                        throw PlayerTrackEnvironment.Error(type: .noTrack, track: nil)
                    }
                }
                do {
                    return try await task.value
                }
            }
            .mapError { $0 as! PlayerTrackEnvironment.Error }
            .receive(on: DispatchQueue.main, options: nil)
            .eraseToEffect()
}

If I'll change the implementation and unwrap the Task and just execute it on the main thread everything is okay.

Any idea what I'm doing wrong?

[mbrandonw]

In general it is not recommended to access the view store's state from inside a .sink on its publisher. This is because the publisher emits on will change rather than did change, and so the view store's state may differ from what is handed to the .sink closure. Note that this is not a design choice TCA makes, but rather how @Published fields and SwiftUI work.

So, rather than doing:

viewStore.publisher
  .sink { _ in 
    self.update()
  }

func update() {
  viewStore.someField
}

You should pass around the view store's emission explicitly:

viewStore.publisher
  .sink { state in 
    self.update(state: state)
  }

func update(state: State) {
  state.someField
}

[rurza]

Oh god, I forgot about it (how the @Published works) :)