Skip to content

Commit e3181d3

Browse files
committed
Merge branch 'main' into fix/chat-infinite-loading-on-removed-member-backbutton-pressed
2 parents 8e51afc + 10d08d2 commit e3181d3

File tree

573 files changed

+19838
-7315
lines changed

Some content is hidden

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

573 files changed

+19838
-7315
lines changed

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

Lines changed: 392 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,398 @@ function Child({ onValueChange }) {
452452

453453
---
454454

455+
### [PERF-11] Optimize data selection and handling
456+
457+
- **Search patterns**: `useOnyx`, `selector`, `.filter(`, `.map(`
458+
459+
- **Condition**: Flag ONLY when ALL of these are true:
460+
461+
- A component uses a broad data structure (e.g., entire object) without selecting specific fields
462+
- This causes unnecessary re-renders when unrelated fields change
463+
- OR unnecessary data filtering/fetching is performed (excluding necessary data, fetching already available data)
464+
465+
**DO NOT flag if:**
466+
467+
- Specific fields are already being selected or the data structure is static
468+
- The filtering is necessary for correct functionality
469+
- The fetched data is required and cannot be derived from existing data
470+
- The function requires the entire object for valid operations
471+
472+
- **Reasoning**: Using broad data structures or performing unnecessary data operations causes excessive re-renders and degrades performance. Selecting specific fields and avoiding redundant operations reduces render cycles and improves efficiency.
473+
474+
Good:
475+
476+
```tsx
477+
function UserProfile({ userId }) {
478+
const [user] = useOnyx(`${ONYXKEYS.USER}${userId}`, {
479+
selector: (user) => ({
480+
name: user?.name,
481+
avatar: user?.avatar,
482+
}),
483+
});
484+
return <Text>{user?.name}</Text>;
485+
}
486+
```
487+
488+
Bad:
489+
490+
```tsx
491+
function UserProfile({ userId }) {
492+
const [user] = useOnyx(`${ONYXKEYS.USER}${userId}`);
493+
// Component re-renders when any user field changes, even unused ones
494+
return <Text>{user?.name}</Text>;
495+
}
496+
```
497+
498+
---
499+
500+
### [PERFORMANCE-12] Prevent memory leaks in components and plugins
501+
502+
- **Search patterns**: `setInterval`, `setTimeout`, `addEventListener`, `subscribe`, `useEffect` with missing cleanup
503+
504+
- **Condition**: Flag ONLY when ALL of these are true:
505+
506+
- A resource (timeout, interval, event listener, subscription, etc.) is created in a component
507+
- The resource is not cleared upon component unmount
508+
- Asynchronous operations are initiated without a corresponding cleanup mechanism
509+
510+
**DO NOT flag if:**
511+
512+
- The resource is cleared properly in a cleanup function (e.g., inside `useEffect` return)
513+
- The resource is not expected to persist beyond the component's lifecycle
514+
- The resource is managed by a library that handles cleanup automatically
515+
- The operation is guaranteed to complete before the component unmounts
516+
517+
- **Reasoning**: Failing to clear resources causes memory leaks, leading to increased memory consumption and potential crashes, especially in long-lived components or components that mount/unmount frequently.
518+
519+
Good:
520+
521+
```tsx
522+
function TimerComponent() {
523+
useEffect(() => {
524+
const intervalId = setInterval(() => {
525+
updateTimer();
526+
}, 1000);
527+
528+
return () => {
529+
clearInterval(intervalId);
530+
};
531+
}, []);
532+
533+
return <Text>Timer</Text>;
534+
}
535+
```
536+
537+
Bad:
538+
539+
```tsx
540+
function TimerComponent() {
541+
useEffect(() => {
542+
const intervalId = setInterval(() => {
543+
updateTimer();
544+
}, 1000);
545+
// Missing cleanup - interval will continue after unmount
546+
}, []);
547+
548+
return <Text>Timer</Text>;
549+
}
550+
```
551+
552+
---
553+
554+
### [CONSISTENCY-1] Avoid platform-specific checks within components
555+
556+
- **Search patterns**: `Platform.OS`, `isAndroid`, `isIOS`, `Platform\.select`
557+
558+
- **Condition**: Flag ONLY when ALL of these are true:
559+
560+
- Platform detection checks (e.g., `Platform.OS`, `isAndroid`, `isIOS`) are present within a component
561+
- The checks lead to hardcoded values or styles specific to a platform
562+
- The component is not structured to handle platform-specific logic through file extensions or separate components
563+
564+
**DO NOT flag if:**
565+
566+
- The logic is handled through platform-specific file extensions (e.g., `index.web.tsx`, `index.native.tsx`)
567+
568+
- **Reasoning**: Mixing platform-specific logic within components increases maintenance overhead, complexity, and bug risk. Separating concerns through dedicated files or components improves maintainability and reduces platform-specific bugs.
569+
570+
Good:
571+
572+
```tsx
573+
// Platform-specific file: Button.desktop.tsx
574+
function Button() {
575+
return <button style={desktopStyles} />;
576+
}
577+
578+
// Platform-specific file: Button.mobile.tsx
579+
function Button() {
580+
return <TouchableOpacity style={mobileStyles} />;
581+
}
582+
```
583+
584+
Bad:
585+
586+
```tsx
587+
function Button() {
588+
const isAndroid = Platform.OS === 'android';
589+
return isAndroid ? (
590+
<TouchableOpacity style={androidStyles} />
591+
) : (
592+
<button style={iosStyles} />
593+
);
594+
}
595+
```
596+
597+
---
598+
599+
### [CONSISTENCY-2] Avoid magic numbers and strings
600+
601+
- **Search patterns**: Hardcoded numbers/strings (context-dependent, look for numeric literals > 1, string literals that aren't obvious)
602+
603+
- **Condition**: Flag ONLY when ALL of these are true:
604+
605+
- Hardcoded strings or numbers are used without documentation or comments
606+
- The value is not defined as a constant elsewhere in the codebase
607+
- The value is not self-explanatory (e.g., `0`, `1`, `Math.PI`)
608+
609+
**DO NOT flag if:**
610+
611+
- The value is self-explanatory (e.g., `Math.PI`, `0`, `1`, `true`, `false`)
612+
- The value is part of configuration or environment variables
613+
- The value is documented with clear comments explaining its purpose
614+
- The value is defined as a named constant in the same file or imported module
615+
616+
- **Reasoning**: Magic numbers and strings reduce code readability and maintainability. Replacing them with named constants or documented values improves clarity and makes future changes easier.
617+
618+
Good:
619+
620+
```tsx
621+
const MAX_RETRY_ATTEMPTS = 3;
622+
const API_TIMEOUT_MS = 5000;
623+
624+
function fetchData() {
625+
if (attempts < MAX_RETRY_ATTEMPTS) {
626+
return apiCall({ timeout: API_TIMEOUT_MS });
627+
}
628+
}
629+
```
630+
631+
Bad:
632+
633+
```tsx
634+
function fetchData() {
635+
if (attempts < 3) {
636+
return apiCall({ timeout: 5000 });
637+
}
638+
}
639+
```
640+
641+
---
642+
643+
### [CONSISTENCY-3] Eliminate code duplication
644+
645+
- **Search patterns**: Similar code patterns, repeated logic (context-dependent analysis)
646+
647+
- **Condition**: Flag ONLY when ALL of these are true:
648+
649+
- Code contains duplicated logic, constants, or components in multiple locations
650+
- The duplicated code performs similar operations or serves the same purpose
651+
- The duplicated code is not abstracted into a reusable function or component
652+
- There is no justification for the duplication
653+
654+
**DO NOT flag if:**
655+
656+
- The duplicated code serves distinct purposes or has different requirements
657+
- The code is intentionally duplicated for performance reasons or due to external constraints
658+
- The duplication is in test or mock code
659+
- The duplication is a temporary measure with a plan for refactoring
660+
661+
- **Reasoning**: Code duplication increases maintenance overhead, raises bug risk, and complicates the codebase. Consolidating similar logic into reusable functions or components adheres to the DRY principle, making code easier to maintain and understand.
662+
663+
Good:
664+
665+
```tsx
666+
function formatCurrency(amount: number, currency: string) {
667+
return new Intl.NumberFormat('en-US', {
668+
style: 'currency',
669+
currency,
670+
}).format(amount);
671+
}
672+
673+
function TransactionList({ transactions }) {
674+
return transactions.map(t => formatCurrency(t.amount, t.currency));
675+
}
676+
677+
function SummaryCard({ total }) {
678+
return <Text>{formatCurrency(total, 'USD')}</Text>;
679+
}
680+
```
681+
682+
Bad:
683+
684+
```tsx
685+
function TransactionList({ transactions }) {
686+
return transactions.map(t =>
687+
new Intl.NumberFormat('en-US', {
688+
style: 'currency',
689+
currency: t.currency,
690+
}).format(t.amount)
691+
);
692+
}
693+
694+
function SummaryCard({ total }) {
695+
return (
696+
<Text>
697+
{new Intl.NumberFormat('en-US', {
698+
style: 'currency',
699+
currency: 'USD',
700+
}).format(total)}
701+
</Text>
702+
);
703+
}
704+
```
705+
706+
---
707+
708+
### [CONSISTENCY-4] Eliminate unused and redundant props
709+
710+
- **Search patterns**: Component prop definitions, unused props in destructuring
711+
712+
- **Condition**: Flag ONLY when ALL of these are true:
713+
714+
- A component defines props that are not referenced in its implementation
715+
- The prop is not conditionally used or part of a larger interface
716+
- The prop is not prepared for future use or part of an ongoing refactor
717+
718+
**DO NOT flag if:**
719+
720+
- Props are conditionally used or part of a larger interface
721+
- Props are prepared for future use or part of an ongoing refactor
722+
- The prop is necessary for functionality or future extensibility
723+
- The prop is redundant but serves a distinct purpose (e.g., backward compatibility)
724+
725+
- **Reasoning**: Unused props increase component complexity and maintenance overhead. Simplifying component interfaces improves code clarity and makes the component API easier to understand.
726+
727+
Good:
728+
729+
```tsx
730+
type ButtonProps = {
731+
title: string;
732+
onPress: () => void;
733+
disabled?: boolean;
734+
};
735+
736+
function Button({ title, onPress, disabled = false }: ButtonProps) {
737+
return (
738+
<TouchableOpacity onPress={onPress} disabled={disabled}>
739+
<Text>{title}</Text>
740+
</TouchableOpacity>
741+
);
742+
}
743+
```
744+
745+
Bad:
746+
747+
```tsx
748+
type ButtonProps = {
749+
title: string;
750+
onPress: () => void;
751+
unusedProp: string; // Never used in component
752+
anotherUnused: number; // Never used in component
753+
};
754+
755+
function Button({ title, onPress }: ButtonProps) {
756+
return (
757+
<TouchableOpacity onPress={onPress}>
758+
<Text>{title}</Text>
759+
</TouchableOpacity>
760+
);
761+
}
762+
```
763+
764+
---
765+
766+
### [CONSISTENCY-5] Justify ESLint rule disables
767+
768+
- **Search patterns**: `eslint-disable`, `eslint-disable-next-line`, `eslint-disable-line`
769+
770+
- **Condition**: Flag ONLY when ALL of these are true:
771+
772+
- An ESLint rule is disabled (via `eslint-disable`, `eslint-disable-next-line`, etc.)
773+
- The disable statement lacks an accompanying comment explaining the reason
774+
775+
**DO NOT flag if:**
776+
777+
- The disablement is justified with a clear comment explaining why the rule is disabled
778+
779+
- **Reasoning**: ESLint rule disables without justification can mask underlying issues and reduce code quality. Clear documentation ensures team members understand exceptions, promoting better maintainability.
780+
781+
Good:
782+
783+
```tsx
784+
// eslint-disable-next-line react-hooks/exhaustive-deps
785+
// Dependencies are intentionally omitted - this effect should only run on mount
786+
useEffect(() => {
787+
initializeComponent();
788+
}, []);
789+
```
790+
791+
Bad:
792+
793+
```tsx
794+
// eslint-disable-next-line react-hooks/exhaustive-deps
795+
useEffect(() => {
796+
initializeComponent();
797+
}, []);
798+
```
799+
800+
---
801+
802+
### [CONSISTENCY-5] Ensure proper error handling
803+
804+
- **Search patterns**: `try`, `catch`, `async`, `await`, `Promise`, `.then(`, `.catch(`
805+
806+
- **Condition**: Flag ONLY when ALL of these are true:
807+
808+
- Error handling logic exists but errors are not logged or handled appropriately
809+
- OR error states are not communicated to the user or developer clearly
810+
- OR a critical function (e.g., API call, authentication, data mutation) lacks error handling
811+
812+
**DO NOT flag if:**
813+
814+
- Errors are logged and handled properly with user feedback
815+
- Errors are intentionally suppressed with clear justification
816+
- Error handling is managed by a higher-level function or middleware
817+
- The operation is non-critical and errors are acceptable to ignore
818+
819+
- **Reasoning**: Proper error handling prevents silent failures, enhances debuggability, and improves user experience. Failing to handle errors can lead to crashes, data loss, and confusion for both developers and users.
820+
821+
Good:
822+
823+
```tsx
824+
async function submitForm(data: FormData) {
825+
try {
826+
await API.submit(data);
827+
showSuccessMessage('Form submitted successfully');
828+
} catch (error) {
829+
Log.error('Form submission failed', error);
830+
showErrorMessage('Failed to submit form. Please try again.');
831+
}
832+
}
833+
```
834+
835+
Bad:
836+
837+
```tsx
838+
async function submitForm(data: FormData) {
839+
await API.submit(data);
840+
// No error handling - failures are silent
841+
showSuccessMessage('Form submitted successfully');
842+
}
843+
```
844+
845+
---
846+
455847
## Instructions
456848

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

0 commit comments

Comments
 (0)