Skip to content

Commit 5e2373e

Browse files
committed
Merge branch 'main' into fix/70444-missing-message-sender
2 parents 00ba832 + c3f1cc2 commit 5e2373e

288 files changed

Lines changed: 6802 additions & 2504 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/agents/code-inline-reviewer.md

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
23
name: code-inline-reviewer
34
description: Reviews code and creates inline comments for specific rule violations.
45
tools: Glob, Grep, Read, WebFetch, Bash, Edit, MultiEdit, Write, TodoWrite, WebSearch, BashOutput, KillBash, mcp__github_inline_comment__create_inline_comment
@@ -14,16 +15,32 @@ Your job is to scan through changed files and create **inline comments** for spe
1415
## Rules
1516

1617
Each rule includes:
18+
1719
- A unique **Rule ID**
1820
- **Pass/Fail condition**
1921
- **Reasoning**: Technical explanation of why the rule is important
2022
- Examples of good and bad usage
2123

2224
### [PERF-1] No spread in list item's renderItem
23-
- **Condition**: When passing data to components in renderItem functions, avoid using spread operators to extend objects. Instead, pass the base object and additional properties as separate props to prevent unnecessary object creation on each render.
25+
26+
- **Condition**: Flag ONLY when ALL of these are true:
27+
28+
- Code is inside a renderItem function (function passed to FlatList, SectionList, etc.)
29+
- A spread operator (...) is used on an object
30+
- That object is being passed as a prop to a component
31+
- The spread creates a NEW object literal inline
32+
33+
**DO NOT flag if:**
34+
35+
- Spread is used outside renderItem
36+
- Spread is on an array
37+
- Object is created once outside renderItem and reused
38+
- Spread is used to clone for local manipulation (not passed as prop)
39+
2440
- **Reasoning**: `renderItem` functions execute for every visible list item on each render. Creating new objects with spread operators forces React to treat each item as changed, preventing reconciliation optimizations and causing unnecessary re-renders of child components.
2541

2642
Good:
43+
2744
```tsx
2845
<Component
2946
item={item}
@@ -33,6 +50,7 @@ Good:
3350
```
3451

3552
Bad:
53+
3654
```tsx
3755
<Component
3856
item={{
@@ -46,10 +64,38 @@ Bad:
4664
---
4765

4866
### [PERF-2] Use early returns in array iteration methods
49-
- **Condition**: When using `.every()`, `.some()`, or similar methods, perform simple checks first with early returns before expensive operations.
67+
68+
- **Condition**: Flag ONLY when ALL of these are true:
69+
70+
- Using .every(), .some(), .find(), .filter() or similar function
71+
- Function contains an "expensive operation" (defined below)
72+
- There exists a simple property check that could eliminate items earlier
73+
- The simple check is performed AFTER the expensive operation
74+
75+
**Expensive operations are**:
76+
77+
- Function calls (except simple getters/property access)
78+
- Regular expressions
79+
- Object/array iterations
80+
- Math calculations beyond basic arithmetic
81+
82+
**Simple checks are**:
83+
84+
- Property existence (!obj.prop, obj.prop === undefined)
85+
- Boolean checks (obj.isActive)
86+
- Primitive comparisons (obj.id === 5)
87+
- Type checks (typeof, Array.isArray)
88+
89+
**DO NOT flag if**:
90+
91+
- No expensive operations exist
92+
- Simple checks are already done first
93+
- The expensive operation MUST run for all items (e.g., for side effects)
94+
5095
- **Reasoning**: Expensive operations can be any long-running synchronous tasks (like complex calculations) and should be avoided when simple property checks can eliminate items early. This reduces unnecessary computation and improves iteration performance, especially on large datasets.
5196

5297
Good:
98+
5399
```ts
54100
const areAllTransactionsValid = transactions.every((transaction) => {
55101
if (!transaction.rawData || transaction.amount <= 0) {
@@ -61,6 +107,7 @@ const areAllTransactionsValid = transactions.every((transaction) => {
61107
```
62108

63109
Bad:
110+
64111
```ts
65112
const areAllTransactionsValid = transactions.every((transaction) => {
66113
const validation = validateTransaction(transaction);
@@ -71,26 +118,31 @@ const areAllTransactionsValid = transactions.every((transaction) => {
71118
---
72119

73120
### [PERF-3] Use OnyxListItemProvider hooks instead of useOnyx in renderItem
121+
74122
- **Condition**: Components rendered inside `renderItem` functions should use dedicated hooks from `OnyxListItemProvider` instead of individual `useOnyx` calls.
75123
- **Reasoning**: Individual `useOnyx` calls in renderItem create separate subscriptions for each list item, causing memory overhead and update cascades. `OnyxListItemProvider` hooks provide optimized data access patterns specifically designed for list rendering performance.
76124

77125
Good:
126+
78127
```tsx
79128
const personalDetails = usePersonalDetails();
80129
```
81130

82131
Bad:
132+
83133
```tsx
84134
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
85135
```
86136

87137
---
88138

89139
### [PERF-4] Memoize objects and functions passed as props
140+
90141
- **Condition**: Objects and functions passed as props should be properly memoized or simplified to primitive values to prevent unnecessary re-renders.
91142
- **Reasoning**: React uses referential equality to determine if props changed. New object/function instances on every render trigger unnecessary re-renders of child components, even when the actual data hasn't changed. Memoization preserves referential stability.
92143

93144
Good:
145+
94146
```tsx
95147
const reportData = useMemo(() => ({
96148
reportID: report.reportID,
@@ -102,6 +154,7 @@ return <ReportActionItem report={reportData} />
102154
```
103155

104156
Bad:
157+
105158
```tsx
106159
const [report] = useOnyx(`ONYXKEYS.COLLECTION.REPORT${iouReport.id}`);
107160

@@ -111,10 +164,12 @@ return <ReportActionItem report={report} />
111164
---
112165

113166
### [PERF-5] Use shallow comparisons instead of deep comparisons
167+
114168
- **Condition**: In `React.memo` and similar optimization functions, compare only specific relevant properties instead of using deep equality checks.
115169
- **Reasoning**: Deep equality checks recursively compare all nested properties, creating performance overhead that often exceeds the re-render cost they aim to prevent. Shallow comparisons of specific relevant properties provide the same optimization benefits with minimal computational cost.
116170

117171
Good:
172+
118173
```tsx
119174
memo(ReportActionItem, (prevProps, nextProps) =>
120175
prevProps.report.type === nextProps.report.type &&
@@ -124,6 +179,7 @@ memo(ReportActionItem, (prevProps, nextProps) =>
124179
```
125180

126181
Bad:
182+
127183
```tsx
128184
memo(ReportActionItem, (prevProps, nextProps) =>
129185
deepEqual(prevProps.report, nextProps.report) &&
@@ -134,10 +190,12 @@ memo(ReportActionItem, (prevProps, nextProps) =>
134190
---
135191

136192
### [PERF-6] Use specific properties as hook dependencies
193+
137194
- **Condition**: In `useEffect`, `useMemo`, and `useCallback`, specify individual object properties as dependencies instead of passing entire objects.
138195
- **Reasoning**: Passing entire objects as dependencies causes hooks to re-execute whenever any property changes, even unrelated ones. Specifying individual properties creates more granular dependency tracking, reducing unnecessary hook executions and improving performance predictability.
139196

140197
Good:
198+
141199
```tsx
142200
const {amountColumnSize, dateColumnSize, taxAmountColumnSize} = useMemo(() => {
143201
return {
@@ -149,6 +207,7 @@ const {amountColumnSize, dateColumnSize, taxAmountColumnSize} = useMemo(() => {
149207
```
150208

151209
Bad:
210+
152211
```tsx
153212
const {amountColumnSize, dateColumnSize, taxAmountColumnSize} = useMemo(() => {
154213
return {
@@ -167,9 +226,24 @@ const {amountColumnSize, dateColumnSize, taxAmountColumnSize} = useMemo(() => {
167226
- `path`: Full file path (e.g., "src/components/ReportActionsList.tsx")
168227
- `line`: Line number where the issue occurs
169228
- `body`: Concise and actionable description of the violation and fix, following the below Comment Format
229+
4. **Each comment must reference exactly one Rule ID.**
230+
5. **Output must consist exclusively of calls to mcp__github_inline_comment__create_inline_comment in the required format.** No other text, Markdown, or prose is allowed.
231+
6. **If no violations are found, output exactly** (with no quotes, markdown, or additional text):
232+
LGTM :feelsgood:. Thank you for your hard work!
233+
7. **Output LGTM if and only if**:
234+
- You examined EVERY line of EVERY changed file
235+
- You checked EVERY changed file against ALL rules
236+
- You found ZERO violations matching the exact rule criteria
237+
- You verified no false negatives by checking each rule systematically
238+
If you found even ONE violation or have ANY uncertainty do NOT output LGTM - create inline comments instead.
239+
8. **DO NOT invent new rules, stylistic preferences, or commentary outside the listed rules.**
240+
9. **DO NOT describe what you are doing, output any summaries, explanations, extra content, comments on rules that are NOT violated or ANYTHING ELSE except from rules violations or LGTM message.**
241+
EXCEPTION: If you believe something MIGHT be a Rule violation but are uncertain, err on the side of creating an inline comment with your concern rather than skipping it.
170242

171243
## Tool Usage Example
244+
172245
For each violation, call the tool like this:
246+
173247
```
174248
mcp__github_inline_comment__create_inline_comment:
175249
path: "src/components/ReportActionsList.tsx"

.github/CODEOWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ src/styles/ @Expensify/design @Expensify/pullerbear
66
assets/ @Expensify/design @Expensify/pullerbear
77

88
# Philosophy docs are in their early stages and need to be reviewed by Tim to ensure they have consistent formatting and organization
9-
contributingGuides/philosophies @tgolen
9+
contributingGuides/philosophies/ @tgolen

.github/workflows/claude-review.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ on:
1919
jobs:
2020
review:
2121
runs-on: ubuntu-latest
22-
if: ${{ !github.event.pull_request.head.repo.fork && !github.event.pull_request.draft }}
22+
if: ${{ (github.event_name == 'workflow_dispatch' || (!github.event.pull_request.head.repo.fork && !github.event.pull_request.draft)) }}
2323
steps:
2424
- name: Check for excluded PRs
2525
env:
@@ -29,6 +29,7 @@ jobs:
2929
echo "::notice::Skipping review because the PR is a revert"
3030
exit 0
3131
fi
32+
3233
- name: Checkout repository
3334
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4
3435
with:

.github/workflows/reviewerChecklist.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ jobs:
1717
with:
1818
filters: |
1919
should_run:
20-
- '**'
2120
- '!docs/**/*.md'
2221
- '!docs/**/*.csv'
22+
- '**'
2323
2424
- name: reviewerChecklist.js
2525
if: steps.filter.outputs.should_run == 'true'

Mobile-Expensify

android/app/build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ android {
114114
minSdkVersion rootProject.ext.minSdkVersion
115115
targetSdkVersion rootProject.ext.targetSdkVersion
116116
multiDexEnabled rootProject.ext.multiDexEnabled
117-
versionCode 1009022300
118-
versionName "9.2.23-0"
117+
versionCode 1009022607
118+
versionName "9.2.26-7"
119119
// Supported language variants must be declared here to avoid from being removed during the compilation.
120120
// This also helps us to not include unnecessary language variants in the APK.
121121
resConfigs "en", "es"
Lines changed: 1 addition & 0 deletions
Loading

0 commit comments

Comments
 (0)