Repository navigation
Conversation
Contributor
|
|
LukasMod
force-pushed
the
feat/use-onyx-get-snapshot-reader
branch
from
October 7, 2026 06:52
a2330a6 to
c71bc7a
Compare
…at allows reasoned disables
LukasMod
force-pushed
the
feat/use-onyx-get-snapshot-reader
branch
from
October 9, 2026 09:39
c71bc7a to
e470849
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Stacked on #102135
useSnapshotOnyxGet()hook (src/hooks/useSnapshotOnyxGet.ts) returns a reader for Search snapshot keys (CONST.SEARCH.SNAPSHOT_ONYX_KEYS), for use in event handlers. Inside a Search scope the reader reads the key from the activesnapshot_<hash>, the same placeuseOnyxwould, without subscribing. Until nowOnyx.getwas banned on these keys because it would return live data where the component shows the snapshot.SearchResultsProvidernow providesSearchSnapshotHashContext, a plain number set to the active snapshot hash, orundefinedon to-do searches that show live data. The hook reads only this number, so it re-renders only when the search changes. In a manual probe run while scrolling Spend > Expenses, a component on this context rendered once, while one readingSearchQueryContextandSearchResultsContextrendered with each pagination.useOnyxnow reads its snapshot hash fromSearchSnapshotHashContexttoo, instead ofSearchQueryContextandSearchResultsContext, so the snapshot condition lives only inSearchResultsProviderand both hooks resolve the same snapshot. The condition itself is unchanged.useOnyx.tsexportsgetKeyDataand the newisSnapshotCompatibleKey, so both hooks share them.getKeyDatanow acceptsOnyxEntry<SearchResults>.no-unsafe-onyx-readnow tracks readers created withconst getOnyx = useSnapshotOnyxGet(), including local copies and inlineuseSnapshotOnyxGet()(key)calls. A reader call is flagged when the key isn't a snapshot key (useOnyx.getfor those), when the key can't be resolved, or when the call happens during render or in an effect. The reader also can't leave the component: passing it as a prop or argument, returning it, or putting it in an object is flagged, because the rule can't follow it there. Listing it in a hook dependency array is allowed. TheOnyx.getsnapshot-key error now tells the developer to useuseSnapshotOnyxGet().src/hooks/useSnapshotOnyxGet.tsitself is exempt from the key checks only.Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari