Skip to content

fix: result isn't changed when the member collection change v2 #716

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.

Sign up for GitHub

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

Merged
tgolen merged 4 commits into from
Jan 12, 2026
+3 −3
Merged

fix: result isn't changed when the member collection change v2 #716

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cbcb573
fix: result isn't changed when the member collection change v2
nkdengineer Dec 29, 2025
dd4a41c
Update lib/useOnyx.ts
nkdengineer Jan 7, 2026
3dbffba
update explain comment
nkdengineer Jan 8, 2026
344f1ea
Merge branch 'main' into fix/76015
nkdengineer Jan 12, 2026
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
4 changes: 2 additions & 2 deletions lib/useOnyx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@
return result;
}

// We get the value from cache while the first connection to Onyx is being made,
// We get the value from cache while the first connection to Onyx is being made or the key has changed,
// so we can return any cached value right away. After the connection is made, we only
// update `newValueRef` when `Onyx.connect()` callback is fired.
if (isFirstConnectionRef.current || shouldGetCachedValueRef.current) {
if (isFirstConnectionRef.current || shouldGetCachedValueRef.current || key !== previousKey) {
Copy link
Collaborator

@tgolen tgolen Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like there are already a handful of options that should help the original issue, without making this change. useOnyx() has options for:

  • initWithStoredValues
  • allowStaleData
  • allowDynamicKey
  • selectors with a dependency array

Wouldn't there be some combination of those that would fix the original problem in the issue?

Copy link
Collaborator

@tgolen tgolen Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump @nkdengineer ^

Copy link
Contributor Author

@nkdengineer nkdengineer Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check this today

Copy link
Contributor Author

@nkdengineer nkdengineer Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the key from useOnyx is changed, the previousKey's value is returned because we don't update newValueRef.current until the Onyx.connect() is fired. This PR updates the getSnapshot function to get the value from cache immediately when the key is changed.

@tgolen From my investigation, when the key is changed but the subscribe isn't fired, the useOnyx returns the result of the previous key as I explained in the PR's details so I think we can't use the options you mentioned above

Copy link
Collaborator

@tgolen tgolen Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the key is changed but the subscribe isn't fired

What is the cause of this not happening?

Copy link
Contributor Author

@nkdengineer nkdengineer Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgolen It's the correct behavior since the Onyx.connect() need time to fire the callback

Copy link
Collaborator

@tgolen tgolen Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't that what this setting is for?

    /**
     * If set to `true`, data will be retrieved from cache during the first render even if there is a pending merge for the key.
     */
    allowStaleData?: boolean;

Copy link
Contributor Author

@nkdengineer nkdengineer Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgolen Yes, that is how isFirstConnectionRef.current works. But in our case, it's not the first render, it's the first time when the key is changed.

Copy link
Collaborator

@tgolen tgolen Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thank you for explaining. I think what I suggest then is to update the comment to explain a little bit more about the "key changed" case. Maybe:

        // We get the value from cache while the first connection to Onyx is being made or if the key has changed,
        // so we can return any cached value right away. For the case where the key has changed, If we don't return the cached value right away, then the UI will show the incorrect (previous) value for a brief period which looks like a UI glitch to the user. After the connection is made, we only
        // update `newValueRef` when `Onyx.connect()` callback is fired.

Copy link
Contributor Author

@nkdengineer nkdengineer Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgolen I updated.

// Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility.
const value = OnyxUtils.tryGetCachedValue(key) as OnyxValue<TKey>;
const selectedValue = memoizedSelector ? memoizedSelector(value) : value;
Expand Down Expand Up @@ -330,7 +330,7 @@

// If `canBeMissing` is set to `false` and the Onyx value of that key is not defined,
// we log an alert so it can be acknowledged by the consumer. Additionally, we won't log alerts
// if there's a `Onyx.clear()` task in progress.

Check warning on line 333 in lib/useOnyx.ts

View workflow job for this annotation

GitHub Actions / lint 

React Hook useCallback has a missing dependency: 'previousKey'. Either include it or remove the dependency array
if (options?.canBeMissing === false && newFetchStatus === 'loaded' && !isOnyxValueDefined && !OnyxCache.hasPendingTask(TASK.CLEAR)) {
Logger.logAlert(`useOnyx returned no data for key with canBeMissing set to false for key ${key}`, {showAlert: true});
}
Expand Down