Skip to content

Commit 8302258

Browse files
committed
fix conflicts
2 parents b586044 + 45472c3 commit 8302258

740 files changed

Lines changed: 32244 additions & 8711 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: 86 additions & 4 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,20 +226,43 @@ 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, create a comment** (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 create LGTM comment - 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, create comments with a summary, explanations, extra content, comments on rules that are NOT violated or ANYTHING ELSE.**
241+
Only inline comments regarding rules violations or general comment with LGTM message are allowed.
242+
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.
170243

171244
## Tool Usage Example
172-
For each violation, call the tool like this:
245+
246+
For each violation, call the mcp__github_inline_comment__create_inline_comment tool like this.
247+
CRITICAL: **DO NOT** use the Bash tool for inline comments:
248+
173249
```
174250
mcp__github_inline_comment__create_inline_comment:
175251
path: "src/components/ReportActionsList.tsx"
176252
line: 128
177253
body: "<Body of the comment according to the Comment Format>"
178254
```
179255

256+
If ZERO violations are found, use the Bash tool to create a top-level PR comment.:
257+
258+
```bash
259+
gh pr comment --body "LGTM :feelsgood:. Thank you for your hard work!"
260+
```
261+
180262
## Comment Format
181263

182264
```
183-
### ❌ **<Rule ID>**
265+
### ❌ <Rule ID> [(docs)](https://github.com/Expensify/App/blob/main/.claude/agents/code-inline-reviewer.md#<Rule ID transformed into a URL hash parameter>)
184266
185267
<Reasoning>
186268

.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
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
name: 'Generate ExpensifyHelp Preview Comment'
2+
description: 'Builds the ExpensifyHelp deployment comment with direct links to updated markdown articles'
3+
inputs:
4+
GITHUB_TOKEN:
5+
description: 'GitHub token with access to list pull request files'
6+
required: true
7+
ROOT_URL:
8+
description: 'ExpensifyHelp preview root URL (e.g., https://123.helpdot.pages.dev)'
9+
required: true
10+
PULL_REQUEST_NUMBER:
11+
description: 'Pull request number to inspect.'
12+
required: true
13+
outputs:
14+
BODY:
15+
description: 'Generated comment body including preview link and updated articles'
16+
runs:
17+
using: 'node20'
18+
main: './index.js'
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import * as core from '@actions/core';
2+
import {context, getOctokit} from '@actions/github';
3+
4+
type OctokitClient = ReturnType<typeof getOctokit>;
5+
6+
const DOCS_DIRECTORY_PREFIX = 'docs/';
7+
const MARKDOWN_EXTENSION = '.md';
8+
const INCLUDED_STATUSES = new Set(['added', 'modified', 'renamed']);
9+
10+
function normalizeAlias(alias: string): string {
11+
return alias.endsWith('/') ? alias : `${alias}/`;
12+
}
13+
14+
function toRoutePath(filename: string): string {
15+
return filename.slice(DOCS_DIRECTORY_PREFIX.length).replace(/\.md$/, '');
16+
}
17+
18+
async function getUpdatedDocRoutes(octokit: OctokitClient, owner: string, repo: string, prNumber: number): Promise<string[]> {
19+
const files = await octokit.paginate(octokit.rest.pulls.listFiles, {
20+
owner,
21+
repo,
22+
// eslint-disable-next-line @typescript-eslint/naming-convention
23+
pull_number: prNumber,
24+
// eslint-disable-next-line @typescript-eslint/naming-convention
25+
per_page: 100,
26+
});
27+
28+
const routes = new Set<string>();
29+
30+
files.forEach((file) => {
31+
const filename = file.filename ?? '';
32+
const status = file.status ?? '';
33+
34+
if (!INCLUDED_STATUSES.has(status)) {
35+
return;
36+
}
37+
38+
if (!filename.startsWith(DOCS_DIRECTORY_PREFIX) || !filename.endsWith(MARKDOWN_EXTENSION)) {
39+
return;
40+
}
41+
42+
routes.add(toRoutePath(filename));
43+
});
44+
45+
return [...routes].sort((a, b) => a.localeCompare(b));
46+
}
47+
48+
async function run(): Promise<void> {
49+
const token = core.getInput('GITHUB_TOKEN', {required: true});
50+
const rootURL = core.getInput('ROOT_URL', {required: true}).trim();
51+
const pullRequestNumber = core.getInput('PULL_REQUEST_NUMBER', {required: true}).trim();
52+
53+
const prNumber = Number(pullRequestNumber);
54+
if (Number.isNaN(prNumber)) {
55+
throw new Error(`Invalid PULL_REQUEST_NUMBER input: ${pullRequestNumber}`);
56+
}
57+
58+
const octokit = getOctokit(token);
59+
const {owner, repo} = context.repo;
60+
61+
const routes = await getUpdatedDocRoutes(octokit, owner, repo, prNumber);
62+
const normalizedRootURL = normalizeAlias(rootURL);
63+
64+
const displayRootURL = normalizedRootURL.replace(/\/$/, '');
65+
let body = `A preview of your ExpensifyHelp changes have been deployed to ${displayRootURL} ⚡️`;
66+
67+
if (routes.length > 0) {
68+
const listItems = routes.map((route) => `- [${route}](${normalizedRootURL}${route})`).join('\n');
69+
body += `\n\n**Updated articles:**\n${listItems}`;
70+
}
71+
72+
core.setOutput('BODY', body);
73+
74+
core.startGroup('Updated ExpensifyHelp routes');
75+
routes.forEach((route) => console.log(route));
76+
core.endGroup();
77+
78+
console.log(`Generated preview comment with ${routes.length} updated article(s)`);
79+
}
80+
81+
if (require.main === module) {
82+
run();
83+
}
84+
85+
export default run;

0 commit comments

Comments
 (0)