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: 148d4d6d84
ℹ️ 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".
148d4d6 to
4826d22
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 536510c2d3
ℹ️ 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: 1ee9f2ab75
ℹ️ 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: cea1ebbf15
ℹ️ 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: 43482ca165
ℹ️ 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: 7fe01ba7c5
ℹ️ 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".
# Conflicts: # package-lock.json # package.json
…xpensify-App into feat/onyx-get-async
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3276dc0d3a
ℹ️ 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".
Rule:
- Trace a callback bound to a name to its use sites, so a selector or a
useMemo factory declared next to the hook rather than inline is still
classified as render (isRenderTimeUsage, getFunctionBinding,
isReferencedAtRenderTime; classifyPosition now threads sourceCode).
- Unwrap TypeScript-only expression wrappers before reading the key, so
`${ONYXKEYS.COLLECTION.X}${id}` as const resolves to its prefix instead
of being reported as noUnresolvableOnyxKey. That cast is the usual form
for a collection member key in this codebase.
- Rename the isReferencedAtRenderTime parameter that shadowed the
module-level rule name, which no-shadow reported as an error.
Tests:
- Cover the named-selector cases against the default parser.
- Add a RuleTester on @typescript-eslint/parser with 13 cases, since the
rule runs on .ts and .tsx in CI but nothing exercised the TS AST.
Docs:
- ONYX-DATA-MANAGEMENT.md: Onyx.get() is asynchronous, so add the missing
await to the eight examples that dereferenced its result directly. The
two render-position examples use `use()` instead, which is what the
noOnyxGetInRender message describes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c352d4b108
ℹ️ 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".
| if (node.init?.type === 'Identifier') { | ||
| const aliasedVariable = getVariableByName(scope, node.init.name); | ||
|
|
||
| if (aliasedVariable && onyxImportBindings.has(aliasedVariable)) { | ||
| trackBinding(node, node.id.name, onyxImportBindings); | ||
| } |
There was a problem hiding this comment.
When a direct read alias is renamed, such as const get = Onyx.get; const read = get;, this identifier branch propagates only onyxImportBindings, so calls through read bypass every diagnostic. The fresh evidence beyond the earlier direct-alias issue is that the first alias is now recorded in readAliases, but a second alias is not; consequently read(ONYXKEYS.COLLECTION.REPORT) is accepted even during render or for a restricted Search snapshot key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
its not a standard practice and not a real case in codebase
1 participant
Explanation of Change
1. Ship
Onyx.get()as a public asynchronous read inreact-native-onyxOnyx.get<TKey>(key: TKey): Promise<OnyxValue<TKey>>on the publicOnyxobject, reachable through the default export oflib/index.ts.OnyxUtils.afterInit:tryGetCachedValuefor a collection,OnyxUtils.getfor a single key, andgetAllKeysplusmultiGetwhen the collection index is cold. These are the readsuseOnyxalready performs, and unlike the internalOnyxUtils.getthis one can return a whole collection. No new storage machinery.Promise, so a component cannotawaitit during render. Reaching a read from render takesuse()or.then()instead, and lint blocks both.Onyx.getdirectly. There is no wrapper: the Search snapshot keys are kept off it byrulesdir/no-unsafe-onyx-read, which derives the banned keys from source and fails closed on a key it cannot resolve.2. Define where
Onyx.getmay be calledAllowed:
voidorPromise<void>useEffectanduseCallbackbodiesrunAfterTransitionscallbacksNot allowed:
useOnyxselectors,useMemobodies,useStateanduseReducerlazy initializers, and bothuseSyncExternalStoresnapshot callbacksOnyx.updatepayload, which keep receiving data through parametersreact native onyx PR: Expensify/react-native-onyx#831
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