Skip to content

Commit 7db3b94

Browse files
Merge branch 'main' into docs-tag-gl-code-column-92692
2 parents dcedc6a + fd57527 commit 7db3b94

6,432 files changed

Lines changed: 124981 additions & 50543 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/settings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"hooks": [
77
{
88
"type": "command",
9-
"command": "FILE=$(jq -r '.tool_input.file_path') && [ -f \"$FILE\" ] && ./node_modules/.bin/prettier --experimental-cli --no-cache --write --ignore-unknown \"$FILE\""
9+
"command": "FILE=$(jq -r '.tool_input.file_path') && [ -f \"$FILE\" ] && ./node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern \"$FILE\""
1010
}
1111
]
1212
}

.claude/skills/coding-standards/SKILL.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ Coding standards for the Expensify App. Each standard is a standalone file in `r
3535
- [PERF-14](rules/perf-14-use-sync-external-store.md) — Use useSyncExternalStore
3636
- [PERF-15](rules/perf-15-cleanup-async-effects.md) — Clean up async Effects
3737
- [PERF-16](rules/perf-16-guard-double-init.md) — Guard double initialization
38+
- [PERF-17](rules/perf-17-pass-raw-index-on-demand.md) — Pass raw source, index on demand (no pre-built digest)
39+
- [PERF-18](rules/perf-18-use-pre-mount-destination.md) — Use usePreMountDestination for RHP pre-mounting
3840

3941
### Consistency
4042
- [CONSISTENCY-1](rules/consistency-1-no-platform-checks.md) — No platform-specific checks in components
@@ -48,6 +50,9 @@ Coding standards for the Expensify App. Each standard is a standalone file in `r
4850
- [CONSISTENCY-9](rules/consistency-9-file-naming.md) — Name files after what they export
4951
- [CONSISTENCY-10](rules/consistency-10-jsdoc.md) — Follow the JSDoc style guidelines
5052
- [CONSISTENCY-11](rules/consistency-11-no-todo-comments.md) — Track future work in an issue, not a TODO comment
53+
- [CONSISTENCY-12](rules/consistency-12-callback-named-for-action.md) — Name callbacks for what they do, not the event they handle
54+
- [CONSISTENCY-13](rules/consistency-13-document-props.md) — Document component props with a JSDoc block comment
55+
- [CONSISTENCY-14](rules/consistency-14-new-file-header.md) — Non-trivial new files start with a header description
5156

5257
### Clean React Patterns
5358
- [CLEAN-REACT-PATTERNS-0](rules/clean-react-0-compiler.md) — React Compiler compliance

.claude/skills/coding-standards/rules/clean-react-0-compiler.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ title: React Compiler compliance
77

88
### Reasoning
99

10-
React Compiler is enabled in this codebase (`babel-plugin-react-compiler` runs first in both webpack and metro configs). It automatically memoizes components and hooks at the AST level — analyzing data flow, tracking dependencies, and inserting fine-grained caching that is more precise than any hand-written `useMemo`, `useCallback`, or `React.memo`.
10+
React Compiler is enabled in this codebase (`babel-plugin-react-compiler` runs first in both Rspack and metro configs). It automatically memoizes components and hooks at the AST level — analyzing data flow, tracking dependencies, and inserting fine-grained caching that is more precise than any hand-written `useMemo`, `useCallback`, or `React.memo`.
1111

1212
Manual memoization is therefore:
1313

.claude/skills/coding-standards/rules/clean-react-1-composition-over-config.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ function Section({title, renderSubtitle, renderTitle, overlayContent}: SectionPr
213213

214214
```tsx
215215
<Table data={items} columns={columns}>
216-
<Table.SearchBar />
217216
<Table.Header />
218217
<Table.Body />
219218
</Table>
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
ruleId: CONSISTENCY-12
3+
title: Name callbacks for what they do, not the event they handle
4+
---
5+
6+
## [CONSISTENCY-12] Name callbacks for what they do, not the event they handle
7+
8+
### Reasoning
9+
10+
Per `contributingGuides/STYLE.md`, a callback method should be named for the action it performs, not for the event slot it happens to be wired to. `toggleReport` tells the reader what happens; `onIconClick` only tells you where it was attached and has to be re-read to learn what it does. Naming by behavior keeps handlers reusable across triggers and searchable by intent. The `on*` / `handle*` naming belongs on the JSX prop, not on the function definition.
11+
12+
### Incorrect
13+
14+
```tsx
15+
function ReportHeader() {
16+
const onIconClick = () => Report.toggleReport(reportID);
17+
const handleButtonPress = () => Navigation.dismissModal();
18+
19+
return <Icon onPress={onIconClick} />;
20+
}
21+
```
22+
23+
### Correct
24+
25+
```tsx
26+
function ReportHeader() {
27+
const toggleReport = () => Report.toggleReport(reportID);
28+
const dismissModal = () => Navigation.dismissModal();
29+
30+
return <Icon onPress={toggleReport} />;
31+
}
32+
```
33+
34+
---
35+
36+
### Review Metadata
37+
38+
Flag ONLY when ALL of these are true:
39+
40+
- The changed code **declares** a function (function declaration or arrow/function assigned to a `const`/`let`) whose name matches `^(on|handle)[A-Z]` and describes the triggering event (e.g. `onIconClick`, `handleButtonPress`, `onRowClick`)
41+
- The name refers to the event slot rather than the action performed, and a behavior-based name (`toggleReport`, `dismissModal`) would be clearer
42+
43+
**DO NOT flag if:**
44+
45+
- The `on*` / `handle*` name appears only as a JSX prop assignment or call site (e.g. `onPress={toggleReport}`) rather than as the function's own declaration
46+
- The function genuinely represents a named event in an event system or implements an external API/interface that dictates the name (e.g. a required `onSuccess` callback prop, `onLayout`, DOM event handler contracts)
47+
- The handler is a prop being passed through/forwarded rather than defined here
48+
- The file is a test or story
49+
50+
**Search Patterns** (hints for reviewers):
51+
- `const on[A-Z]` / `const handle[A-Z]` / `function on[A-Z]` / `function handle[A-Z]`
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
ruleId: CONSISTENCY-13
3+
title: Document component props with a JSDoc block comment
4+
---
5+
6+
## [CONSISTENCY-13] Document component props with a JSDoc block comment
7+
8+
### Reasoning
9+
10+
Per `contributingGuides/STYLE.md`, every component prop is documented with a `/** ... */` block comment above it so its purpose is clear at the definition site. This rule enforces the *presence* of that documentation when a new props type is added with no documented props at all. It is the companion to CONSISTENCY-10, which enforces JSDoc *style* and catches the mixed cases (a `//` comment on a prop, or an undocumented prop sitting next to documented siblings). Together they ensure every prop carries a `/** */` block.
11+
12+
### Incorrect
13+
14+
```tsx
15+
type ButtonProps = {
16+
isDisabled: boolean;
17+
onPress: () => void;
18+
};
19+
```
20+
21+
### Correct
22+
23+
```tsx
24+
type ButtonProps = {
25+
/** Whether the button is disabled */
26+
isDisabled: boolean;
27+
28+
/** Called when the button is pressed */
29+
onPress: () => void;
30+
};
31+
```
32+
33+
---
34+
35+
### Review Metadata
36+
37+
Flag ONLY when ALL of these are true:
38+
39+
- The changed code adds (or newly introduces the members of) a component props type/interface - a `type`/`interface` whose name ends in `Props`
40+
- It declares one or more of its own props
41+
- **None** of those props has a `/** ... */` block comment above it
42+
43+
**DO NOT flag if:**
44+
45+
- At least one prop in the type is already documented with `/** */` (the mixed/undocumented-sibling and `//`-comment cases belong to CONSISTENCY-10, not here - avoid double-flagging)
46+
- The type only re-exports, extends, intersects, or spreads props from a base type documented elsewhere and declares no new members of its own
47+
- The props are inherited from a shared base type
48+
- The file is a test or story
49+
50+
**Search Patterns** (hints for reviewers):
51+
- Added `type ...Props = {` / `interface ...Props {` blocks whose members have no preceding `/** */`
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
ruleId: CONSISTENCY-14
3+
title: Non-trivial new files start with a header description
4+
---
5+
6+
## [CONSISTENCY-14] Non-trivial new files start with a header description
7+
8+
### Reasoning
9+
10+
When a new file is added it should carry a short description of what it does and/or why it is needed at the top, unless the code is self explanatory. A one-line header comment on a new module, component, hook, or util orients the next reader immediately instead of forcing them to reverse-engineer the file's purpose from its exports.
11+
12+
### Incorrect
13+
14+
A new `src/libs/CardUtils/deriveLimit.ts` that opens straight into code:
15+
16+
```tsx
17+
import type {Card} from '@src/types/onyx';
18+
19+
function deriveLimit(card: Card): number {
20+
// ...
21+
}
22+
```
23+
24+
### Correct
25+
26+
```tsx
27+
/**
28+
* Helpers for deriving a card's spend limit from its Onyx data.
29+
*/
30+
import type {Card} from '@src/types/onyx';
31+
32+
function deriveLimit(card: Card): number {
33+
// ...
34+
}
35+
```
36+
37+
---
38+
39+
### Review Metadata
40+
41+
Flag ONLY when ALL of these are true:
42+
43+
- The change **adds a new file** (not a modification of an existing one) under `src/**`
44+
- The file contains non-trivial logic (more than a few lines; not a pure re-export or a single constant)
45+
- Its first non-import, non-directive token is not a comment - there is no header comment describing the file
46+
47+
**DO NOT flag if:**
48+
49+
- The file is a barrel / `index.*` re-export file, a platform-suffixed variant (`.ios`/`.android`/`.native`/`.web`) of a file documented in its base variant, or a type-only declaration file
50+
- The file is short and self explanatory (e.g. a tiny constant, a one-line util whose name says everything)
51+
- The file is a test, story, snapshot, or generated/config file
52+
- A header comment is already present
53+
54+
**Search Patterns** (hints for reviewers):
55+
- Newly added files under `src/**` whose first line is `import`/`export` with no leading `/** */` or `//` header
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
---
2+
ruleId: PERF-17
3+
title: Avoid intermediate lookup layers
4+
---
5+
6+
## [PERF-17] Avoid intermediate lookup layers
7+
8+
### Reasoning
9+
10+
Building a derived lookup structure (`new Set`/`new Map`/`Object.fromEntries` or a hand-built index) from a collection you already hold - just to answer membership/per-element checks downstream - is O(n) construction, often re-run every render, for something the callee can index O(1) straight from the raw source. Pass the raw collection and do the lookup on demand inside the callee instead.
11+
12+
### Examples
13+
14+
#### Incorrect
15+
16+
```tsx
17+
// Archived reports in last-accessed navigation (ReportRouteParamHandler.tsx)
18+
// Builds an intermediate lookup layer from the full collection on every render.
19+
const archivedReportsIDSet = useArchivedReportsIDSet();
20+
21+
useFocusEffect(
22+
useCallback(() => {
23+
const report = findLastAccessedReport(ignoreDomainRooms, isOpenOnAdminRoom, undefined, archivedReportsIDSet);
24+
}, [archivedReportsIDSet, ignoreDomainRooms, isOpenOnAdminRoom]),
25+
);
26+
```
27+
28+
#### Correct
29+
30+
```tsx
31+
// Pass raw collection and index on demand inside the callee.
32+
const [reportNameValuePairs] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS);
33+
34+
useFocusEffect(
35+
useCallback(() => {
36+
const report = findLastAccessedReport(ignoreDomainRooms, isOpenOnAdminRoom, undefined, reportNameValuePairs);
37+
}, [reportNameValuePairs, ignoreDomainRooms, isOpenOnAdminRoom]),
38+
);
39+
40+
// inside findLastAccessedReport:
41+
const reportNVP = reportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`];
42+
const isArchived = isArchivedReport(reportNVP);
43+
```
44+
45+
#### Incorrect
46+
47+
```tsx
48+
// Distance rates membership in workspace settings (PolicyDistanceRatesPage.tsx)
49+
const selectableRates = useMemo(() => buildSelectableRates(customUnitRates), [customUnitRates]);
50+
51+
// Unnecessary intermediate lookup layer from data already keyed by ID.
52+
const rateIDs = useMemo(() => new Set(Object.keys(selectableRates)), [selectableRates]);
53+
54+
const isSelectable = rateIDs.has(transaction?.comment?.customUnit?.customUnitRateID);
55+
```
56+
57+
#### Correct
58+
59+
```tsx
60+
const selectableRates = useMemo(() => buildSelectableRates(customUnitRates), [customUnitRates]);
61+
62+
const rateID = transaction?.comment?.customUnit?.customUnitRateID;
63+
const isSelectable = !!rateID && !!selectableRates[rateID];
64+
```
65+
66+
---
67+
68+
### Review Metadata
69+
70+
**Flag when ALL of these are true:**
71+
72+
- A derived lookup structure (`new Set`/`new Map`/`Object.fromEntries`, or a hand-built index object) is constructed from a collection/array the code already holds.
73+
- It is built eagerly - at component scope or on every render (e.g. a hook or `useMemo` over the whole collection) - rather than behind the branch that needs it.
74+
- Its only purpose is membership/lookup checks answerable O(1) directly against the raw source, i.e. the raw source is already keyed (object / `Map` / Onyx collection).
75+
76+
Increase confidence (not required):
77+
78+
- The callee runs conditionally/lazily (for example, inside a `useCallback`/event handler, `useFocusEffect`, or a `useState(() => ...)` initializer), so the intermediate lookup layer is rebuilt on every render even when the callee and lookup never run.
79+
80+
**DO NOT flag if:**
81+
82+
- Genuine repeated querying against the intermediate lookup layer is confirmed (for example, `.has(...)` inside a loop or multiple distinct lookup sites). If that evidence is not visible in the diff, confirm it by searching the callee or changed file.
83+
- The raw source isn't already available to the callee and would otherwise need its own expensive fetch.
84+
- The structure meaningfully reshapes data (not just a membership/lookup index) and the callee needs the whole thing.
85+
- The raw source is an array: a `Set`/`Map` that turns O(n) `.includes`/scan membership into O(1) `.has` is a real precompute win, not a violation.
86+
87+
**Search Patterns** (hints for reviewers):
88+
- `new Set\(`
89+
- `new Map\(`
90+
- `Object\.fromEntries`
91+
- `\.has\(`

0 commit comments

Comments
 (0)