Merge branch 'main' into fix/public-room-message-send-after-restart

Author: nabi-ebrahimi

.claude/schemas/code-review-output.json

@@ -436,6 +436,7 @@ Flag when ANY of these are true:
 - A component receives a large set of props that collectively configure its appearance and behavior (e.g., dialog with `isOpen`, `title`, `tabs`, `activeTab`, `onTabChange`, `onSave`, `onReset`, `values`)
 - These props could be broken into independent composable blocks: Provider manages state, sub-components render independently
 - The component becomes a "configuration object consumer" rather than a composition of building blocks
+- When counting props for this case, exclude **coordination props** that match the Case 3 carve-out below (state setters or callbacks structurally shared across multiple sibling children). Count only props that configure the component's own appearance or behavior. If the remaining prop count no longer reads as monolithic, do not flag.
 
 **Case 4 — Config-array driven rendering:**
 - Statically-known items (finite, fixed at development time) are encoded as a data array of config objects
@@ -468,6 +469,10 @@ In all cases, the rule applies to: **new components**, **new features added to e
 - The `ReactNode` prop is `children` itself — `children` is the foundation of composition, not configuration
 - The render function is a **list component callback** (`renderItem` on `FlatList`, `DraggableList`) — these are framework requirements
 - The render function receives **per-item runtime data** from a dynamic collection (e.g., `renderSuggestionMenuItem(item, index)`) — this is list-style rendering, not slot configuration
+- **(Case 3 only)** Props exist to **coordinate shared state or callbacks across sibling children** whose state must live in the common parent. This is legitimate state lifting, not configuration. The coordination role must be **structurally detectable** in the diff or surrounding code — apply ONLY when ONE of these is true:
+    - The **same state setter** (e.g., `setSelectedId`) is passed to two or more sibling children, OR
+    - The **same callback** (e.g., `onItemPress`) is consumed by two or more sibling children
+  A prop passed to a single child, or a prop merely described as "shared" without a visible second consumer, does NOT qualify — this qualifier exists to prevent the carve-out from becoming an argument-shaped escape hatch for any prop.
 
 **Search Patterns** (hints for reviewers):
 - **Case 1**: `should\w+`, `can\w+`, `enable`, `disable` (boolean flag prefixes in prop types)

.claude/scripts/addPrReaction.sh

@@ -93,36 +93,50 @@ In this example:
 
 #### Incorrect (internal render helper functions)
 
-- Internal `render*` functions signal the component owns multiple rendering responsibilities that should be separate components
-- They close over the entire component scope — the React Compiler cannot independently memoize them
-- They hide the component's render tree — the return statement doesn't show what actually renders
-- The fix is to extract each helper into its own component with explicit props
+- Internal render helpers become a problem when they carry hooks, side effects, or non-trivial business logic that belong in a dedicated component
+- They become a problem when they build deeply nested JSX inline, hiding the real render tree from the return statement
+- They become a problem when they close over mutable parent state (e.g., values from `useState`/`useReducer`) and bake that state into the JSX they produce
+- React Compiler handles thin delegation wrappers (e.g., a function that picks a child component based on a prop) without issue — the rule targets helpers that are doing real work, not ones that just route to already-extracted children
+- The fix for the problematic cases is to extract the helper into its own component with explicit props
 
 ```tsx
-function ForYouSection() {
-    const theme = useTheme();
+function ReportDetailsPage({report, policy}: Props) {
+    const [draftNote, setDraftNote] = useState('');
     const {translate} = useLocalize();
-    const {submitCount, approveCount} = useTodos();
-
-    // ❌ Internal render helper — closes over everything, hides structure
-    const renderTodoItems = () => (
-        <View style={styles.todoContainer}>
-            {todoItems.map(({key, icon, ...rest}) => (
-                <BaseWidgetItem key={key} icon={icon} {...rest} />
-            ))}
-        </View>
-    );
 
-    // ❌ Another render helper — branching logic buried in a function
-    const renderContent = () => {
-        if (isLoadingApp) {
-            return <ActivityIndicator size="large" />;
-        }
-        return hasAnyTodos ? renderTodoItems() : <EmptyState />;
+    // ❌ Heavy internal helper — closes over mutable state, runs business logic,
+    //    and builds deeply nested JSX inline. This is what the rule targets.
+    const renderNoteSection = () => {
+        const trimmed = draftNote.trim();
+        const isValid = trimmed.length > 0 && trimmed.length <= CONST.NOTE.MAX_LENGTH;
+        const participants = ReportUtils.getParticipantsList(report, policy);
+        const canEdit = ReportUtils.canEditReportNote(report, participants);
+
+        return (
+            <View style={styles.section}>
+                <View style={styles.sectionHeader}>
+                    <Text style={styles.sectionTitle}>{translate('notes.title')}</Text>
+                    {canEdit && (
+                        <View style={styles.sectionActions}>
+                            <Button
+                                text={translate('common.save')}
+                                isDisabled={!isValid}
+                                onPress={() => Report.saveNote(report.reportID, trimmed)}
+                            />
+                        </View>
+                    )}
+                </View>
+                <TextInput
+                    value={draftNote}
+                    onChangeText={setDraftNote}
+                    editable={canEdit}
+                    multiline
+                />
+            </View>
+        );
     };
 
-    // The return statement hides the actual render tree
-    return <WidgetContainer>{renderContent()}</WidgetContainer>;
+    return <ScreenWrapper>{renderNoteSection()}</ScreenWrapper>;
 }
 ```
 
@@ -189,9 +203,9 @@ Flag when a component, hook, or utility aggregates multiple unrelated responsibi
 - Unrelated state variables are interdependent or updated together
 - Logic mixes data fetching, navigation, UI state, and lifecycle behavior in one place
 - Removing one piece of functionality requires careful untangling from others
-- Component defines internal `render*` functions or arrow functions that return JSX and calls them in its return statement (e.g., `const renderContent = () => ...`, `{renderContent()}`)
-- These functions close over the component's entire scope, preventing the React Compiler from independently memoizing them
-- The component's return statement calls these helpers instead of showing the render tree directly
+- An internal function that returns JSX contains hooks, side effects, or non-trivial business logic (data shaping, permission checks, validation, etc.) that should live in its own component
+- An internal function that returns JSX builds deeply nested JSX inline, obscuring the component's render tree
+- An internal function that returns JSX closes over mutable parent state (e.g., values from `useState`/`useReducer`) and embeds that state in the JSX it produces
 
 **What counts as "unrelated":**
 - Group by responsibility (what the code does), NOT by timing (when it runs)
@@ -203,6 +217,8 @@ Flag when a component, hook, or utility aggregates multiple unrelated responsibi
 - Effects are extracted into focused custom hooks with single responsibilities (e.g., `useDebugShortcut`, `usePriorityMode`) — inline `useEffect` calls are a code smell and should be named hooks
 - The internal function is a **callback or event handler** (e.g., `handlePress`, `onSubmit`), not a render helper — only functions that return JSX qualify
 - The internal function is a **single early return** for a guard clause (e.g., `if (!data) return <EmptyState />;` at the top of the component) — simple guards in the component body are not render helpers
+- The internal function is a **switch/conditional helper that delegates to already-extracted child components** (e.g., `const renderItem = () => type === 'foo' ? <FooItem /> : <BarItem />`) — React Compiler memoizes these thin wrappers correctly
+- The internal function (or inline `.map()` callback) **iterates over data and renders an already-extracted child component** per item (e.g., `items.map((item) => <Row key={item.id} item={item} />)`) — the per-item work happens inside the extracted child
 
 **Search Patterns** (hints for reviewers):
 - `useEffect`

.claude/scripts/createInlineComment.sh

@@ -7,7 +7,11 @@ title: Justify ESLint rule disables
 
 ### Reasoning
 
-ESLint rule disables without justification can mask underlying issues and reduce code quality. Clear documentation ensures team members understand exceptions, promoting better maintainability.
+`eslint-disable` comments completely hide violations from the seatbelt baseline — they will never be fixed unless the comment is removed. Because of this, they should only be used for **permanent** suppressions where you are certain the rule genuinely does not apply to that specific case and you don't expect the violation to ever be fixed.
+
+For **temporary** suppressions (e.g. the fix requires a large refactor that is out of scope), prefer widening the seatbelt baseline with `SEATBELT_INCREASE` instead — that keeps the violation visible so it can still be fixed later. See [`LINTING.md`](../../../../contributingGuides/LINTING.md) for full guidance on choosing between the two approaches.
+
+When `eslint-disable` is used, a justification comment is required so that reviewers and future maintainers understand why the rule was deliberately suppressed.
 
 ### Incorrect
 
@@ -37,6 +41,10 @@ Flag ONLY when ALL of these are true:
 - An ESLint rule is disabled (via `eslint-disable`, `eslint-disable-next-line`, etc.)
 - The disable statement lacks an accompanying comment explaining the reason
 
+**Also consider flagging if:**
+
+- `eslint-disable` is used for what appears to be a temporary/convenience suppression — in those cases, suggest `SEATBELT_INCREASE` as the preferred alternative (see [`LINTING.md`](../../../../contributingGuides/LINTING.md))
+
 **DO NOT flag if:**
 
 - The disablement is justified with a clear comment explaining why the rule is disabled

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

@@ -9,6 +9,8 @@ title: Avoid useEffect chains
 
 Chains of effects create complex dependencies, timing issues, and unnecessary renders. Logic should be restructured to avoid interdependent effects.
 
+Chains are wasteful only when the bridge state has no observable render role. If the intermediate state drives UI - conditional rendering, props passed to a rendered child, or gating a subtree - the re-render is the purpose of the state, not waste, and PERF-9 does not apply.
+
 ### Incorrect
 
 ```tsx
@@ -58,10 +60,12 @@ Flag ONLY when ALL of these are true:
 
 - Multiple `useEffect` hooks exist
 - One effect's state update triggers another effect
+- The intermediate state has no render-driving role - it is not referenced in JSX for conditional rendering, prop pass-through to a rendered child, or gating a subtree
 - Logic could be combined or computed instead
 
 **DO NOT flag if:**
 
+- The intermediate state is consumed as a render-driving dependency in JSX - e.g., conditional rendering (`{state && <X />}`), prop pass-through to a child the user observes, or gating an entire subtree. In those cases the re-render is the purpose of the state, not waste.
 - Effects handle different concerns (e.g., one for setup, one for event listening)
 - Effects depend on external events that can't be combined
 - Separation of concerns requires multiple effects

.claude/skills/coding-standards/rules/clean-react-4-no-side-effect-spaghetti.md

@@ -91,7 +91,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
     - [ ] I verified that comments were added to code that is not self explanatory
     - [ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
     - [ ] I verified any copy / text shown in the product is localized by adding it to `src/languages/*` files and using the [translation method](https://github.com/Expensify/App/blob/4510fc76bbf5df699a2575bfb49a276af90f3ed7/src/components/LocaleContextProvider.tsx#L80)
-      - [ ] If any non-english text was added/modified, I used [JaimeGPT](https://chatgpt.com/g/g-2dgOQl5VM-english-to-spanish-translator-aka-jaimegpt) to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
     - [ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the [localization methods](https://github.com/Expensify/App/blob/4510fc76bbf5df699a2575bfb49a276af90f3ed7/src/components/LocaleContextProvider.tsx#L116-L123)
     - [ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
     - [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
@@ -155,4 +154,4 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
 
 <!-- add screenshots or videos here -->
 
-</details>
+</details>