Remove asynchronous scheduling from Onyx subscriber notifications #82871
Description
Coming from a proposal here
Background
Onyx notifies subscribed React components whenever data changes using scheduleSubscriberUpdate()/scheduleNotifyCollectionSubscribers() functions. These notifications are deferred through prepareSubscriberUpdate(), which wraps every notification in setTimeout(0) + Promise.resolve() + Promise.all().
This scheduling was originally part of a custom batching system that used React's unstable_batchedUpdates to group subscriber updates into a single render cycle. That batching was removed in react-native-onyx#689 since React 18 introduced automatic batching, and the custom implementation only worked for the deprecated withOnyx HOC.
However, the macro/microtask barrier was left in place because removing it exposed a race condition between subscribeToKey and scheduleSubscriberUpdate/scheduleNotifyCollectionSubscribers:
- When Onyx.connect() is called, inside subscribeToKey(), the initial data delivery to the subscriber is deferred by fetching data from cache/storage before calling sendDataToConnection().
- Without the macro/microtask barrier, if data is updated right after subscribing to a key and scheduleSubscriberUpdate()/scheduleNotifyCollectionSubscribers() are called, they deliver fresh data to the subscriber synchronously. Then the cache/storage read from subscribeToKey() resolves and sendDataToConnection() overwrites it with stale data because it doesn't know the subscriber was already notified.
The macro/microtask barrier masks this race condition rather than fixing it. It also forces every write operation to propagate an updatePromise through the merge pipeline, adding complexity with no remaining benefit.
Problem
When Onyx processes data updates, every write operation allocates setTimeout timers and Promise objects to defer subscriber notifications. This macro/microtask barrier no longer serves its original batching purpose and exists only to mask an underlying race condition in which sendDataToConnection() can overwrite fresh subscriber data with stale storage results.
Solution
Fix the race condition in sendDataToConnection() and remove the macro/microtask barrier entirely:
- Fix sendDataToConnection() to detect already-notified subscribers. Replace the reference equality check (valueToPass === lastValue) with a simple existence check (lastConnectionCallbackData.has(mapping.subscriptionID)). If a subscriber already has an entry in lastConnectionCallbackData, it was notified by a synchronous keyChanged()/keysChanged() call during the current call stack, so sendDataToConnection() skips the initial delivery, preventing stale data from overwriting fresh data.
- Track collection subscriber notifications in keyChanged(). When keyChanged() notifies a collection subscriber, record the delivered data via lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection), as it's already done for individual key subscribers. Without this, collection subscribers wouldn't be recognized as already-notified by sendDataToConnection().
- Remove prepareSubscriberUpdate(), nextMacrotaskPromise, and the wrapper functions scheduleSubscriberUpdate()/scheduleNotifyCollectionSubscribers(). With the race condition properly fixed, the macro/microtask barrier is no longer needed. All call sites now invoke keyChanged() and keysChanged() directly.
This fixes the race condition instead of masking it with a timing workaround, simplifies the codebase, and eliminates unnecessary setTimeout/Promise allocations on every write operation.
Draft PR: Expensify/react-native-onyx#724
Issue Owner
Current Issue Owner: @VickyStashMetadata
Metadata
Assignees
Labels
Type
Projects
Status