Potential race condition on Combine's @Published property wrapper

70 Views Asked by At

I found some unexpected behavior when using @Published to listen for view model's updates. Here's what I found:

// My View Model Class
class NotificationsViewModel {
    // MARK: - Properties

    @Published private(set) var notifications = [NotificationData]()

    // MARK: - APIs

    func fetchAllNotifications() {
        Task {
            do {
                // This does a network call to get all the notifications.
                notifications = try await NotificationsService.shared.getAllNotifications()
            } catch {
                printError(error)
            }
        }
    }
}

class NotificationsViewController: UIViewController {
    private let viewModel = NotificationsViewModel()
    // Here are some more properties..

    override func viewDidLoad() {
        super.viewDidLoad()

        // This sets up the UI, such as adding a table view.
        configureUI()
        // This binds the current VC to the View Model.
        bindToViewModel()
    }

    func bindToViewModel() {
        viewModel.fetchAllNotifications()
        viewModel.$notifications.receive(on: DispatchQueue.main).sink { [weak self] notifs in
            if self?.viewModel.notifications.count != notifs.count {
                print("debug: notifs.count - \(notifs.count), viewModel.notifications.count - \(self?.viewModel.notifications.count)")
            }
            self?.tableView.reloadData()
        }.store(in: &cancellables)
    }
}

Surprisingly, sometimes the table view is empty, even if there are notifications for my user. After some debugging, I found when I try to reload the table view after viewModel.$notifications notifies my VC about the updates, the actual viewModel.notifications property didn't get updated, while the notifs in the subscription receive handler is correctly updated.

A sample output of my issue is: debug: notifs.count - 8, viewModel.notifications.count - Optional(0)

Is this due to some race condition of @Published property? And what is the best practice of solving this issue? I know I can add a didSet to notifications and imperatively asks my VC to refresh itself, or simply call self?.tableView.reloadData() in the next main runloop. But neither of them look clean.

1

There are 1 best solutions below

2
Scott Thompson On

The notification of an @Published property occurs in the willSet handler for the property. That means that the value of the property does not actually change until after all the notifications are sent.

Inside of your sink call where you ask for self?.viewModel.notifications you are getting the previous value of notifications, not the new value that is to be stored in the property. The new value is not available in your sink method.

If you want to use Combine to post notifications, use a CurrentValueSubject instead of an @Published property. @Published is really aimed at syntax sugar for SwiftUI not as a general publish/subscribe mechanism for models.

I would recommend using UITableViewDiffableDataSource and not using reloadData. Reloading the table kicks off a lot of work, all the work the OS did to get ready to draw is re-done.