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..8b6968024993 --- /dev/null +++ b/.claude/skills/coding-standards/rules/clean-react-6-no-componentprops.md @@ -0,0 +1,51 @@ +--- +ruleId: CLEAN-REACT-6 +title: Import the exported prop type instead of ComponentProps +--- + +## [CLEAN-REACT-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..2c330639502b --- /dev/null +++ b/.claude/skills/coding-standards/rules/clean-react-8-no-class-components.md @@ -0,0 +1,47 @@ +--- +ruleId: CLEAN-REACT-8 +title: Use function components, not class components +--- + +## [CLEAN-REACT-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..7982a0f998c6 --- /dev/null +++ b/.claude/skills/coding-standards/rules/clean-react-9-no-proptypes.md @@ -0,0 +1,60 @@ +--- +ruleId: CLEAN-REACT-9 +title: Use TypeScript types, not propTypes or defaultProps +--- + +## [CLEAN-REACT-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