Skip to content

Commit 11f2866

Browse files
authored
Merge branch 'Expensify:main' into allowAddingFeedForCollectCSV
2 parents 085a9c3 + 99867a7 commit 11f2866

325 files changed

Lines changed: 3180 additions & 12001 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: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,212 @@ memo(ReportActionItem, (prevProps, nextProps) =>
246246
)
247247
```
248248

249+
---
250+
251+
### [PERF-6] Derive state from props
252+
253+
- **Condition**: Flag when useEffect updates state based on props or other state, when the value could be computed directly
254+
255+
- **Reasoning**: Computing derived values directly in the component body ensures they're always synchronized with props/state and avoids unnecessary re-renders.
256+
257+
Good:
258+
259+
```tsx
260+
function Form() {
261+
const [firstName, setFirstName] = useState('Taylor');
262+
const [lastName, setLastName] = useState('Swift');
263+
264+
// ✅ Good: calculated during rendering
265+
const fullName = firstName + ' ' + lastName;
266+
}
267+
```
268+
269+
Bad:
270+
271+
```tsx
272+
function Form() {
273+
const [firstName, setFirstName] = useState('Taylor');
274+
const [lastName, setLastName] = useState('Swift');
275+
276+
// 🔴 Avoid: redundant state and unnecessary Effect
277+
const [fullName, setFullName] = useState('');
278+
useEffect(() => {
279+
setFullName(firstName + ' ' + lastName);
280+
}, [firstName, lastName]);
281+
}
282+
```
283+
284+
---
285+
286+
### [PERF-7] Control component resets via key prop
287+
288+
- **Condition**:
289+
- Flag when useEffect resets all or most component state when a prop changes
290+
- Should use `key` prop instead to reset the entire component
291+
292+
- **Reasoning**: Using `key` prop for full resets is more React-idiomatic. When a prop changes and you need to reset all component state, the `key` prop causes React to unmount and remount the component, automatically resetting all state without needing useEffect.
293+
294+
Good:
295+
296+
```tsx
297+
function ProfilePage({ userId }) {
298+
return <ProfileView key={userId} userId={userId} />;
299+
}
300+
301+
function ProfileView({ userId }) {
302+
const [comment, setComment] = useState('');
303+
const [rating, setRating] = useState(0);
304+
// Component resets when userId changes due to key prop
305+
}
306+
```
307+
308+
Bad:
309+
310+
```tsx
311+
// 🔴 Avoid: resetting all state with useEffect
312+
function ProfilePage({ userId }) {
313+
return <ProfileView userId={userId} />;
314+
}
315+
316+
function ProfileView({ userId }) {
317+
const [comment, setComment] = useState('');
318+
const [rating, setRating] = useState(0);
319+
320+
useEffect(() => {
321+
setComment(''); // Reset when userId changes
322+
setRating(0);
323+
}, [userId]);
324+
}
325+
```
326+
327+
---
328+
329+
### [PERF-8] Handle events in event handlers
330+
331+
- **Condition**: Flag when useEffect responds to user events that should be handled in event handlers
332+
333+
- **Reasoning**: Event handlers provide immediate response and clearer code flow. useEffect adds unnecessary render cycles and makes the relationship between user action and response less clear.
334+
335+
Good:
336+
337+
```tsx
338+
function BuyButton({ productId, onBuy }) {
339+
function handleClick() {
340+
// ✅ Good: handle event directly in event handler
341+
onBuy();
342+
showNotification('Item purchased!');
343+
}
344+
345+
return <button onClick={handleClick}>Buy</button>;
346+
}
347+
```
348+
349+
Bad:
350+
351+
```tsx
352+
function BuyButton({ productId, onBuy }) {
353+
const [isBuying, setIsBuying] = useState(false);
354+
355+
// 🔴 Avoid: handling events in useEffect
356+
useEffect(() => {
357+
if (isBuying) {
358+
onBuy();
359+
showNotification('Item purchased!');
360+
}
361+
}, [isBuying, onBuy]);
362+
363+
return <button onClick={() => setIsBuying(true)}>Buy</button>;
364+
}
365+
```
366+
367+
---
368+
369+
### [PERF-9] Avoid useEffect chains
370+
371+
- **Condition**: Flag when multiple useEffects form a chain where one effect's state update triggers another effect
372+
373+
- **Reasoning**: Chains of effects create complex dependencies, timing issues, and unnecessary renders. Logic should be restructured to avoid interdependent effects.
374+
375+
Good:
376+
377+
```tsx
378+
function Form() {
379+
const [firstName, setFirstName] = useState('');
380+
const [lastName, setLastName] = useState('');
381+
382+
// ✅ Good: compute derived values directly
383+
const fullName = firstName + ' ' + lastName;
384+
const isValid = firstName.length > 0 && lastName.length > 0;
385+
386+
return (
387+
<form>
388+
<input value={firstName} onChange={e => setFirstName(e.target.value)} />
389+
<input value={lastName} onChange={e => setLastName(e.target.value)} />
390+
{isValid && <button>Submit</button>}
391+
</form>
392+
);
393+
}
394+
```
395+
396+
Bad:
397+
398+
```tsx
399+
function Form() {
400+
const [firstName, setFirstName] = useState('');
401+
const [lastName, setLastName] = useState('');
402+
const [fullName, setFullName] = useState('');
403+
const [isValid, setIsValid] = useState(false);
404+
405+
// 🔴 Avoid: chain of effects
406+
useEffect(() => {
407+
setFullName(firstName + ' ' + lastName);
408+
}, [firstName, lastName]);
409+
410+
useEffect(() => {
411+
setIsValid(fullName.length > 0);
412+
}, [fullName]);
413+
}
414+
```
415+
416+
---
417+
418+
### [PERF-10] Communicate with parent components without useEffect
419+
420+
- **Condition**: Flag when useEffect calls parent callbacks to communicate state changes or pass data to parent components
421+
422+
- **Reasoning**: Parent-child communication should not use useEffect. Instead, lift the state up to the parent component and pass it down as props. This follows React's unidirectional data flow pattern, eliminates synchronization issues, reduces unnecessary renders, and makes the data flow clearer. Use useEffect only when synchronizing with external systems, not for parent-child communication.
423+
424+
Good:
425+
426+
```tsx
427+
// Lifting state up
428+
function Parent() {
429+
const [value, setValue] = useState('');
430+
return <Child value={value} onChange={setValue} />;
431+
}
432+
433+
function Child({ value, onChange }) {
434+
return <input value={value} onChange={e => onChange(e.target.value)} />;
435+
}
436+
```
437+
438+
Bad:
439+
440+
```tsx
441+
// 🔴 Avoid: passing data via useEffect
442+
function Child({ onValueChange }) {
443+
const [value, setValue] = useState('');
444+
445+
useEffect(() => {
446+
onValueChange(value);
447+
}, [value, onValueChange]);
448+
449+
return <input value={value} onChange={e => setValue(e.target.value)} />;
450+
}
451+
```
452+
453+
---
454+
249455
## Instructions
250456

251457
1. **First, get the list of changed files and their diffs:**

.claude/scripts/checkReactCompilerOptimization.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ function findPlatformVariants(resolvedPath: string): PlatformVariant[] {
6767
const variants: PlatformVariant[] = [];
6868

6969
// Platform suffixes to check
70-
const platforms = ['native', 'ios', 'android', 'web', 'desktop'];
70+
const platforms = ['native', 'ios', 'android', 'web'];
7171

7272
const ext = path.extname(basename);
7373
const nameWithoutExt = path.basename(basename, ext);

.github/actions/composite/setupNode/action.yml

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,25 @@ runs:
1919
shell: bash
2020
run: jq 'del(.version, .packages[""].version)' package-lock.json > normalized-package-lock.json
2121

22-
- uses: actions/setup-node@cdca7365b2dadb8aad0a33bc7601856ffabcc48e #v4
22+
- id: setup-node
23+
# v6.1.0
24+
uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f
2325
with:
2426
node-version-file: '.nvmrc'
25-
cache: npm
27+
cache: 'npm'
2628
cache-dependency-path: normalized-package-lock.json
2729

2830
- id: cache-node-modules
29-
# v4
30-
uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57
31+
# v5.0.1
32+
uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb
3133
with:
3234
path: node_modules
3335
key: ${{ inputs.IS_HYBRID_BUILD == 'true' && format('{0}-node-modules-{1}', runner.os, hashFiles('package-lock.json', 'patches/**', 'Mobile-Expensify/patches/**')) || format('{0}-node-modules-{1}', runner.os, hashFiles('package-lock.json', 'patches/**'))}}
3436

3537
- id: cache-old-dot-node-modules
3638
if: inputs.IS_HYBRID_BUILD == 'true'
37-
# v4
38-
uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57
39+
# v5.0.1
40+
uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb
3941
with:
4042
path: Mobile-Expensify/node_modules
4143
key: ${{ runner.os }}-node-modules-${{ hashFiles('Mobile-Expensify/package-lock.json', 'Mobile-Expensify/patches/**') }}

.github/scripts/addPrReaction.sh

Lines changed: 0 additions & 16 deletions
This file was deleted.

.github/tsconfig.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
"paths": {
1212
"@assets/*": ["../assets/*"],
1313
"@components/*": ["../src/components/*"],
14-
"@desktop/*": ["../desktop/*"],
1514
"@github/*": ["../.github/*"],
1615
"@hooks/*": ["../src/hooks/*"],
1716
"@libs/*": ["../src/libs/*"],

.github/workflows/cspell.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ jobs:
1919
run: jq 'del(.version, .packages[""].version)' package-lock.json > normalized-package-lock.json
2020

2121
- name: Restore cspell cache
22-
# v4
23-
uses: actions/cache/restore@1bd1e32a3bdc45362d1e726936510720a7c30a57
22+
# v5.0.1
23+
uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb
2424
with:
2525
path: .cspellcache
2626
key: cspell-${{ runner.os }}-${{ hashFiles('cspell.json', 'normalized-package-lock.json') }}-${{ github.sha }}
@@ -48,7 +48,7 @@ jobs:
4848

4949
- name: Save cspell cache
5050
# v4
51-
uses: actions/cache/save@1bd1e32a3bdc45362d1e726936510720a7c30a57
51+
uses: actions/cache/save@9255dc7a253b0ccc959486e2bca901246202afeb
5252
if: always()
5353
with:
5454
path: .cspellcache

.github/workflows/deploy.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,8 @@ jobs:
327327
run: bundle install
328328

329329
- name: Cache Pod dependencies
330-
# v4
331-
uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57
330+
# v5.0.1
331+
uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb
332332
id: pods-cache
333333
with:
334334
path: Mobile-Expensify/iOS/Pods

.github/workflows/deployNewHelp.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ jobs:
5555

5656
# Install Node for _scripts/*.js
5757
- name: Set up Node.js
58-
# v4
59-
uses: actions/setup-node@cdca7365b2dadb8aad0a33bc7601856ffabcc48e
58+
# v6.0.1
59+
uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f
6060
with:
6161
node-version-file: '.nvmrc'
6262

.github/workflows/lint-changed.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ jobs:
6060
run: jq 'del(.version, .packages[""].version)' package-lock.json > normalized-package-lock.json
6161

6262
- name: Restore ESLint cache
63-
# v4
64-
uses: actions/cache/restore@1bd1e32a3bdc45362d1e726936510720a7c30a57
63+
# v5.0.1
64+
uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb
6565
with:
6666
path: node_modules/.cache/eslint-changed
6767
key: ${{ runner.os }}-eslint-changed-${{ hashFiles('eslint.changed.config.*', 'normalized-package-lock.json') }}-${{ github.sha }}
@@ -81,8 +81,8 @@ jobs:
8181
fi
8282
8383
- name: Save ESLint cache
84-
# v4
85-
uses: actions/cache/save@1bd1e32a3bdc45362d1e726936510720a7c30a57
84+
# v5.0.1
85+
uses: actions/cache/save@9255dc7a253b0ccc959486e2bca901246202afeb
8686
if: always()
8787
with:
8888
path: node_modules/.cache/eslint-changed

.github/workflows/lint.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ jobs:
2929
run: jq 'del(.version, .packages[""].version)' package-lock.json > normalized-package-lock.json
3030

3131
- name: Restore ESLint cache
32-
# v4
33-
uses: actions/cache/restore@1bd1e32a3bdc45362d1e726936510720a7c30a57
32+
# v5.0.1
33+
uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb
3434
with:
3535
path: node_modules/.cache/eslint
3636
key: ${{ runner.os }}-eslint-${{ hashFiles('eslint.config.*', 'normalized-package-lock.json') }}-${{ github.sha }}
@@ -52,8 +52,8 @@ jobs:
5252
CI: true
5353

5454
- name: Save ESLint cache
55-
# v4
56-
uses: actions/cache/save@1bd1e32a3bdc45362d1e726936510720a7c30a57
55+
# v5.0.1
56+
uses: actions/cache/save@9255dc7a253b0ccc959486e2bca901246202afeb
5757
if: always()
5858
with:
5959
path: node_modules/.cache/eslint

0 commit comments

Comments
 (0)