Add public Onyx.get - #831
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef3f683cb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36f7695d07
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
0fa6e3b to
d3e0b52
Compare
86e84a0 to
b747f62
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b747f6257d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Once init has resolved, afterInit runs the action synchronously and setWithRetry and multiSetWithRetry reach cache.set before returning, so an un-awaited set or multiSet is already visible to the next read. Only merge and mergeCollection apply later. Three tests pin which of the four writes a following read sees, so the documented contract cannot drift again.
The paragraph opened four sentences in a row with A-noun-is-verb, buried the actionable rule inside a passive clause, and leaned on "at all" for emphasis. merge() and set() are the actors here, so name them.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Details
Adds
Onyx.get(key), a public one-shot asynchronous read on theOnyxobject, so callers can read a key without opening a subscription.What it does
Onyx.get<TKey>(key: TKey): Promise<ReadonlyDeep<OnyxValue<TKey>>>(lib/Onyx.ts:634), registered on theOnyxobject (lib/Onyx.ts:665) and therefore reachable through the default export oflib/index.ts.OnyxUtils.afterInit, so a call made beforeOnyx.init()has finished resolves with the hydrated value instead ofundefined.OnyxUtils.get(key)and normalisesnulltoundefined.OnyxUtils.getAllKeys()plusOnyxUtils.multiGet()over the matching member keys, which also warms the cache for the next read.OnyxUtils.getcannot return a whole collection, so that gap is what this function closes.OnyxInput(lib/types.ts:266) is wrapped in the same type so a value read withget()can be handed back toset,merge,mergeCollectionandsetCollection. Areadonlyarray is not assignable to a mutable one, so wrapping only the return type would break that round trip.useOnyx()still hands back the same object typed mutable, which is the looser of the two.No new storage machinery is introduced; these are the same reads
useOnyxalready performs.Semantics, documented in the JSDoc and in
API.mdmerge()andmergeCollection()queue changes for a later tick, so a read issued before one resolves will not see it: await the write.set()andmultiSet()reach the cache before returning onceinit()has finished, so their value is visible without awaiting.{}. A collection read on a completely empty store resolves toundefined.undefined, whichsetCollectionandmergeCollectionreject, so a whole-collection read has to drop those members before going back in. Single keys and collection members have no such gap.useOnyx()orOnyx.connectWithoutView()when the value has to stay current.get()is a point-in-time read.Related Issues
GH_LINK
Linked E/App PR
Expensify/App#99667
Automated Tests
tests/unit/onyxTest.tsgains adescribe('get')block with 22 cases covering:undefined{}once the store holds datauseOnyxwould read, and agreeing with a subscription on a key the cache no longer holdsmerge()ormergeCollection()set()writes, and an un-awaitedmultiSet()Four more cases in
Onyx.init > should only execute Onyx methods after initializationcover theafterInitpaths: waiting for initialization for a single key and for a collection, resolvingundefinedafter init when the key has no value, and resolving a collection asundefinedon a completely empty store.tests/types/get.tspins the typing. Mutating a read value stays a type error, and a read value stays accepted bysetandmerge, including one carrying areadonlyarray member. Reverting either the return type or theOnyxInputwrapper failsnpm run test:types, so both halves are covered. The linked E/App PR is the wider check that deeply read-only inputs typecheck against the realKeyValueMapping.tests/perf-test/Onyx.perf-test.tsgains adescribe('get')block with two benchmarks: 1k reads of a whole 10k-member collection of heavy objects, and 10k reads of individual collection members.API.mdis regenerated from the JSDoc.Manual Tests
This is a library API addition with no UI surface, so it is exercised through the linked E/App PR rather than in isolation:
react-native-onyxto this PR's HEAD.Onyx.getstill load their data.useOnyxfor the same keys.Author Checklist
### Related Issuessection above### Linked E/App PRsection above, and verified this change against it (E/App CI passed and manual testing completed)TestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)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