diff --git a/.claude/skills/coding-standards/SKILL.md b/.claude/skills/coding-standards/SKILL.md index 9b89e4336ac6..7136a4341e7a 100644 --- a/.claude/skills/coding-standards/SKILL.md +++ b/.claude/skills/coding-standards/SKILL.md @@ -13,8 +13,9 @@ Coding standards for the Expensify App. Each standard is a standalone file in `r | Category | Prefix | Focus | |----------|--------|-------| | Performance | `PERF-*` | Render optimization, memo patterns, useEffect hygiene, data selection | -| Consistency | `CONSISTENCY-*` | Platform checks, magic values, unused props, ESLint discipline | -| Clean React Patterns | `CLEAN-REACT-PATTERNS-*` | Composition, component ownership, state structure | +| Consistency | `CONSISTENCY-*` | Platform checks, magic values, unused props, ESLint discipline, localization, file naming, JSDoc | +| Clean React Patterns | `CLEAN-REACT-PATTERNS-*` | Composition, component ownership, state structure, prop typing, function components | +| UI | `UI-*` | Loading indicators, scrollable pages, styling conventions | ## Quick Reference @@ -42,6 +43,11 @@ Coding standards for the Expensify App. Each standard is a standalone file in `r - [CONSISTENCY-4](rules/consistency-4-no-unused-props.md) — No unused props - [CONSISTENCY-5](rules/consistency-5-justify-eslint-disable.md) — Justify ESLint disables - [CONSISTENCY-6](rules/consistency-6-proper-error-handling.md) — Proper error handling +- [CONSISTENCY-7](rules/consistency-7-localize-copy.md) — Localize all user-visible copy +- [CONSISTENCY-8](rules/consistency-8-localize-numbers-dates.md) — Localize numbers, amounts, dates and phone numbers +- [CONSISTENCY-9](rules/consistency-9-file-naming.md) — Name files after what they export +- [CONSISTENCY-10](rules/consistency-10-jsdoc.md) — Follow the JSDoc style guidelines +- [CONSISTENCY-11](rules/consistency-11-no-todo-comments.md) — Track future work in an issue, not a TODO comment ### Clean React Patterns - [CLEAN-REACT-PATTERNS-0](rules/clean-react-0-compiler.md) — React Compiler compliance @@ -50,6 +56,15 @@ Coding standards for the Expensify App. Each standard is a standalone file in `r - [CLEAN-REACT-PATTERNS-3](rules/clean-react-3-context-free-contracts.md) — Context-free component contracts - [CLEAN-REACT-PATTERNS-4](rules/clean-react-4-no-side-effect-spaghetti.md) — No side-effect spaghetti - [CLEAN-REACT-PATTERNS-5](rules/clean-react-5-narrow-state.md) — Keep state narrow +- [CLEAN-REACT-PATTERNS-6](rules/clean-react-6-no-componentprops.md) — Import the exported prop type instead of ComponentProps +- [CLEAN-REACT-PATTERNS-7](rules/clean-react-7-no-inline-prop-types.md) — Do not inline prop types on exported components +- [CLEAN-REACT-PATTERNS-8](rules/clean-react-8-no-class-components.md) — Use function components, not class components +- [CLEAN-REACT-PATTERNS-9](rules/clean-react-9-no-proptypes.md) — Use TypeScript types, not propTypes or defaultProps + +### UI +- [UI-1](rules/ui-1-correct-loading-indicator.md) — Use the correct loading indicator based on navigation context +- [UI-2](rules/ui-2-new-page-scrollview.md) — New pages must be scrollable +- [UI-3](rules/ui-3-no-inline-styles.md) — Do not use inline style objects ## Usage diff --git a/.claude/skills/coding-standards/rules/clean-react-6-no-componentprops.md b/.claude/skills/coding-standards/rules/clean-react-6-no-componentprops.md new file mode 100644 index 000000000000..f573a4a610a7 --- /dev/null +++ b/.claude/skills/coding-standards/rules/clean-react-6-no-componentprops.md @@ -0,0 +1,51 @@ +--- +ruleId: CLEAN-REACT-PATTERNS-6 +title: Import the exported prop type instead of ComponentProps +--- + +## [CLEAN-REACT-PATTERNS-6] Import the exported prop type instead of ComponentProps + +### Reasoning + +When you need another component's prop type, import the type the component already exports rather than deriving it with `ComponentProps`. Deriving the type couples callers to the component's implementation, breaks when props are renamed, and hides the intended public contract. Components export their props type explicitly for this purpose. + +### Incorrect + +```tsx +import type {ComponentProps} from 'react'; +import Button from './Button'; + +type Props = { + button: ComponentProps; +}; +``` + +### Correct + +```tsx +import Button from './Button'; +import type {ButtonProps} from './Button'; + +type Props = { + button: ButtonProps; +}; +``` + +--- + +### Review Metadata + +Flag ONLY when ALL of these are true: + +- The changed code uses `ComponentProps` (or `React.ComponentProps`) to obtain a component's props +- That component exports its own props type that could be imported instead + +**DO NOT flag if:** + +- The component is a third-party/library component that does not export a props type +- `ComponentProps` is used on an intrinsic element (e.g. `ComponentProps<'div'>`) +- The code is a test or story + +**Search Patterns** (hints for reviewers): +- `ComponentProps; +} + +export default Avatar; +``` + +### Correct + +```tsx +type AvatarProps = { + /** URL of the avatar image */ + source: string; + + /** Rendered width/height in px */ + size: number; +}; + +function Avatar({source, size}: AvatarProps) { + return ; +} + +export default Avatar; +``` + +--- + +### Review Metadata + +Flag ONLY when ALL of these are true: + +- A component that is exported from the file declares its props as an inline object type literal in the parameter position (e.g. `(props: {a: string})` / `({a}: {a: string})`) +- The component is a React component (returns JSX / is rendered) + +**DO NOT flag if:** + +- The props are already a named `type`/imported type +- The function is a small local helper component used only within the same file and not exported +- The code is a test or story + +**Search Patterns** (hints for reviewers): +- `function [A-Z][A-Za-z]*\([^)]*:\s*\{` (component with inline object param type) +- `}: \{` immediately inside a component signature diff --git a/.claude/skills/coding-standards/rules/clean-react-8-no-class-components.md b/.claude/skills/coding-standards/rules/clean-react-8-no-class-components.md new file mode 100644 index 000000000000..416a7800d4f2 --- /dev/null +++ b/.claude/skills/coding-standards/rules/clean-react-8-no-class-components.md @@ -0,0 +1,47 @@ +--- +ruleId: CLEAN-REACT-PATTERNS-8 +title: Use function components, not class components +--- + +## [CLEAN-REACT-PATTERNS-8] Use function components, not class components + +### Reasoning + +Per `STYLE.md`, class components are deprecated in this codebase. New components must be written as function components using hooks. Class components opt out of the React Compiler, complicate state/lifecycle reasoning, and diverge from the rest of the codebase. + +### Incorrect + +```tsx +class ReportActionItem extends React.Component { + render() { + return {this.props.action.message}; + } +} +``` + +### Correct + +```tsx +function ReportActionItem({action}: ReportActionItemProps) { + return {action.message}; +} +``` + +--- + +### Review Metadata + +Flag ONLY when ALL of these are true: + +- The changed code adds a class that extends `React.Component`/`Component`/`React.PureComponent`/`PureComponent` + +**DO NOT flag if:** + +- The class is an Error subclass, a non-React class, or a data model +- The change only modifies an existing class component (migrating it is out of scope) rather than adding a new one +- The class is an Error Boundary, which still requires a class component in React + +**Search Patterns** (hints for reviewers): +- `extends React.Component` +- `extends Component` +- `extends React.PureComponent` / `extends PureComponent` diff --git a/.claude/skills/coding-standards/rules/clean-react-9-no-proptypes.md b/.claude/skills/coding-standards/rules/clean-react-9-no-proptypes.md new file mode 100644 index 000000000000..c41bd6f62b4e --- /dev/null +++ b/.claude/skills/coding-standards/rules/clean-react-9-no-proptypes.md @@ -0,0 +1,60 @@ +--- +ruleId: CLEAN-REACT-PATTERNS-9 +title: Use TypeScript types, not propTypes or defaultProps +--- + +## [CLEAN-REACT-PATTERNS-9] Use TypeScript types, not propTypes or defaultProps + +### Reasoning + +Per `STYLE.md`, components must type their props with TypeScript and provide defaults via destructuring. The `prop-types` library (`PropTypes.*`, `Component.propTypes`) and `defaultProps` are redundant with the type system, add runtime cost, and are not used in this codebase. + +### Incorrect + +```tsx +import PropTypes from 'prop-types'; + +function Badge({text}) { + return {text}; +} + +Badge.propTypes = { + text: PropTypes.string, +}; + +Badge.defaultProps = { + text: '', +}; +``` + +### Correct + +```tsx +type BadgeProps = { + /** Text shown inside the badge */ + text?: string; +}; + +function Badge({text = ''}: BadgeProps) { + return {text}; +} +``` + +--- + +### Review Metadata + +Flag ONLY when ANY of these is true: + +- The changed code imports `prop-types` or references `PropTypes.` +- It assigns `.propTypes` to a component +- It assigns `.defaultProps` to a function component (use destructuring defaults instead) + +**DO NOT flag if:** + +- The code is a test or story +- `defaultProps` appears on a third-party class component API that requires it + +**Search Patterns** (hints for reviewers): +- `from 'prop-types'` / `PropTypes.` +- `.propTypes =` / `.defaultProps =` diff --git a/.claude/skills/coding-standards/rules/consistency-10-jsdoc.md b/.claude/skills/coding-standards/rules/consistency-10-jsdoc.md new file mode 100644 index 000000000000..fa20a772b536 --- /dev/null +++ b/.claude/skills/coding-standards/rules/consistency-10-jsdoc.md @@ -0,0 +1,66 @@ +--- +ruleId: CONSISTENCY-10 +title: Follow the JSDoc style guidelines +--- + +## [CONSISTENCY-10] Follow the JSDoc style guidelines + +### Reasoning + +Per `STYLE.md`, TypeScript already encodes types, so JSDoc must not repeat them. Do not put types in `@param`/`@returns`, do not use `@private`/`@memberof`/`@implements`/`@enum`/`@override`, and use `@returns` (not `@return`). Omit a `@param` line entirely when it would carry no description. Component props are documented with a `/** ... */` block comment above each prop, not `//` comments. + +### Incorrect + +```tsx +/** + * @param {string} reportID - the report id + * @param {boolean} isArchived + * @return {string} + */ +function getReportName(reportID: string, isArchived: boolean): string { + // ... +} + +type ButtonProps = { + // Whether the button is disabled + isDisabled: boolean; +}; +``` + +### Correct + +```tsx +/** + * @param reportID - the report id + * @returns the human-readable report name + */ +function getReportName(reportID: string, isArchived: boolean): string { + // ... +} + +type ButtonProps = { + /** Whether the button is disabled */ + isDisabled: boolean; +}; +``` + +--- + +### Review Metadata + +Flag ONLY when ANY of these is true: + +- A JSDoc `@param`/`@returns` includes a TypeScript type in braces (e.g. `@param {string}`) +- A JSDoc block uses `@return` instead of `@returns`, or uses `@private`/`@memberof`/`@implements`/`@enum`/`@override` +- A `@param` line has a name but no description (it should be omitted) +- A component prop in a `Props` type is documented with a `//` comment or left undocumented when sibling props use `/** */` blocks + +**DO NOT flag if:** + +- The function is a trivial inline arrow with no JSDoc and self-evident behavior (JSDoc not required) +- The prop is inherited/spread from a shared base type documented elsewhere +- The file is a test or story + +**Search Patterns** (hints for reviewers): +- `@param {` / `@returns {` / `@return ` / `@private` / `@memberof` +- `//` comments directly above members of a `...Props` type diff --git a/.claude/skills/coding-standards/rules/consistency-11-no-todo-comments.md b/.claude/skills/coding-standards/rules/consistency-11-no-todo-comments.md new file mode 100644 index 000000000000..62d2976afa16 --- /dev/null +++ b/.claude/skills/coding-standards/rules/consistency-11-no-todo-comments.md @@ -0,0 +1,46 @@ +--- +ruleId: CONSISTENCY-11 +title: Track future work in a GitHub issue, not a TODO comment +--- + +## [CONSISTENCY-11] Track future work in a GitHub issue, not a TODO comment + +### Reasoning + +Per `contributingGuides/philosophies/OVERENGINEERING.md`, future work should be captured in a GitHub issue, not left as a `TODO`/`FIXME` comment in the code. In-code TODOs are invisible to planning, never prioritized, and rot silently in the codebase. + +### Incorrect + +```tsx +// TODO: handle the offline case here later +function submit() { + return API.write('SubmitReport', params); +} +``` + +### Correct + +The deferred work lives as a GitHub issue on the board - the code carries no marker for it at all: + +```tsx +function submit() { + return API.write('SubmitReport', params); +} +``` + +--- + +### Review Metadata + +Flag ONLY when ALL of these are true: + +- The changed code adds a comment containing `TODO` or `FIXME` +- The comment describes deferred/future work (which belongs in a GitHub issue, not in the code) + +**DO NOT flag if:** + +- `TODO`/`FIXME` appears in a string literal, test fixture, or third-party/generated code rather than an authored code comment +- The token is part of an unrelated identifier (e.g. a variable literally named `todo` in a tasks feature) + +**Search Patterns** (hints for reviewers): +- `// TODO` / `/* TODO` / `// FIXME` diff --git a/.claude/skills/coding-standards/rules/consistency-7-localize-copy.md b/.claude/skills/coding-standards/rules/consistency-7-localize-copy.md new file mode 100644 index 000000000000..9f55c668bc54 --- /dev/null +++ b/.claude/skills/coding-standards/rules/consistency-7-localize-copy.md @@ -0,0 +1,65 @@ +--- +ruleId: CONSISTENCY-7 +title: Localize all user-visible copy +--- + +## [CONSISTENCY-7] Localize all user-visible copy + +### Reasoning + +All copy/text shown in the product must be localized by adding it to the `src/languages/*` files and rendering it through the translation method (`useLocalize`'s `translate`, or `` fed a translated string). Hardcoded user-facing strings cannot be translated and break the localized experience. + +### Incorrect + +```tsx +function SaveButton() { + return diff --git a/src/components/ButtonComposed/context/ButtonContext.ts b/src/components/ButtonComposed/context/ButtonContext.ts index 0ec2d80e1e58..54fba4c3a585 100644 --- a/src/components/ButtonComposed/context/ButtonContext.ts +++ b/src/components/ButtonComposed/context/ButtonContext.ts @@ -2,12 +2,11 @@ import {createContext, useContext} from 'react'; import CONST from '@src/CONST'; import type {ButtonContextValue} from './types'; +/** Fallback used when a Button primitive is rendered outside a `