skill

react-conventions

React-specific code conventions and anti-patterns for JavaScript projects. Detects component structure issues, hook misuse, prop drilling, missing cleanups, and rendering problems that ESLint and eslint-plugin-react-hooks do not catch. Use when analyzing React components, reviewing React code, or auditing a React codebase.

$ curl -fsSL https://skills.reveni.dev/install.sh | bash

React Conventions

These rules target React-specific problems that ESLint, eslint-plugin-react, and eslint-plugin-react-hooks do NOT catch. General code quality issues (duplication, dead code, complexity) are handled by the code-quality skill — do not repeat them here.

Every rule below includes: severity, what to detect, how to detect it, and examples.

Rule: component-size

Severity: MEDIUM

Detect components that are too large. Threshold: >200 lines (non-blank, non-comment) or JSX return block >50 lines.

Detection: Count lines in each component file. If a single component exceeds the threshold, it likely handles too many responsibilities.

Bad:

// src/components/OrderPage/OrderPage.jsx — 350 lines
const OrderPage = () => {
  // 40 lines of state declarations
  // 60 lines of useEffect calls
  // 30 lines of handler functions
  // 20 lines of data transformations
  // 200 lines of JSX
  return (
    <div>
      {/* header, filters, table, pagination, modals, toasts... */}
    </div>
  );
};

Good:

const OrderPage = () => {
  const { orders, filters, pagination } = useOrders();
  return (
    <div>
      <OrderFilters {...filters} />
      <OrderTable orders={orders} />
      <Pagination {...pagination} />
    </div>
  );
};

Report format: Show file path, line count, and suggest splitting into smaller components or extracting logic into custom hooks.

Rule: business-logic-in-view

Severity: HIGH

Detect API calls, data transformations, or complex business logic placed directly inside the component body instead of being extracted to custom hooks or service modules.

Detection: Look for components that contain:

Bad:

const UserList = () => {
  const [users, setUsers] = useState([]);

  useEffect(() => {
    fetch('/api/users')
      .then(res => res.json())
      .then(data => {
        const active = data.filter(u => u.status === 'active');
        const sorted = active.sort((a, b) => b.lastLogin - a.lastLogin);
        const withFullName = sorted.map(u => ({
          ...u,
          fullName: `${u.firstName} ${u.lastName}`,
        }));
        setUsers(withFullName);
      });
  }, []);

  return <ul>{users.map(u => <li key={u.id}>{u.fullName}</li>)}</ul>;
};

Good:

const UserList = () => {
  const { users } = useActiveUsers();
  return <ul>{users.map(u => <li key={u.id}>{u.fullName}</li>)}</ul>;
};
// Logic extracted to src/hooks/useActiveUsers.js

Report format: Show the component name, file:line, what logic should be extracted, and suggest a custom hook or service.

Rule: prop-drilling

Severity: MEDIUM

Detect props passed through 3+ levels of components where intermediate components do not use them.

Detection: Trace a prop from a parent component through its children. If a prop appears in a component's parameter list and is only forwarded to a child (not read or used in any logic or JSX), count it as a passthrough. Flag when a prop passes through 3 or more components that don't use it.

Bad:

// App → Layout → Sidebar → UserMenu → Avatar
// "user" prop passed through Layout and Sidebar without being used

const Layout = ({ user, children }) => (
  <div>
    <Sidebar user={user} />
    {children}
  </div>
);

const Sidebar = ({ user }) => (
  <nav>
    <UserMenu user={user} />
  </nav>
);

Good:

// Use Context — Layout and Sidebar don't need the user prop
const UserMenu = () => {
  const user = useContext(UserContext);
  return <Avatar src={user.avatar} />;
};

Report format: Show the prop name, the chain of components it passes through, and suggest Context or composition.

Rule: index-as-key

Severity: HIGH

Detect usage of array index as key prop in lists where items can be added, removed, reordered, or filtered.

Detection: Find .map((item, index) => <X key={index} or .map((_, i) => <X key={i} patterns. Only flag if the list is dynamic (items come from state or props, not a static array defined outside the component).

Bad:

{orders.map((order, index) => (
  <OrderRow key={index} order={order} />
))}

Good:

{orders.map((order) => (
  <OrderRow key={order.id} order={order} />
))}

Why it matters: Using index as key causes React to reuse DOM nodes incorrectly when items are reordered or removed, leading to stale state in child components, broken animations, and input fields losing their values.

Report format: Show file:line, the .map expression, and suggest using a stable unique identifier from the data.

Rule: missing-cleanup

Severity: HIGH

Detect useEffect hooks that create subscriptions, timers, or event listeners without returning a cleanup function.

Detection: Look for useEffect bodies that contain:

Bad:

useEffect(() => {
  const timer = setInterval(() => {
    setCount(c => c + 1);
  }, 1000);
}, []);

useEffect(() => {
  window.addEventListener('resize', handleResize);
}, []);

Good:

useEffect(() => {
  const timer = setInterval(() => {
    setCount(c => c + 1);
  }, 1000);
  return () => clearInterval(timer);
}, []);

useEffect(() => {
  window.addEventListener('resize', handleResize);
  return () => window.removeEventListener('resize', handleResize);
}, []);

Report format: Show file:line, what resource is created, and that no cleanup is returned.

Rule: derived-state

Severity: MEDIUM

Detect useState for values that can be computed from existing state or props, causing unnecessary state synchronization.

Detection: Look for:

Bad:

const [items, setItems] = useState([]);
const [filteredItems, setFilteredItems] = useState([]);
const [searchTerm, setSearchTerm] = useState('');

useEffect(() => {
  setFilteredItems(items.filter(i => i.name.includes(searchTerm)));
}, [items, searchTerm]);

Good:

const [items, setItems] = useState([]);
const [searchTerm, setSearchTerm] = useState('');

const filteredItems = items.filter(i => i.name.includes(searchTerm));

Why it matters: Derived state causes an extra render cycle (first render with stale value, then re-render with derived value), adds unnecessary complexity, and is a common source of sync bugs.

Report format: Show the redundant useState and the useEffect that syncs it, and suggest computing it inline or with useMemo.

Rule: component-in-component

Severity: HIGH

Detect React components defined inside other components. This causes the inner component to be unmounted and remounted on every render of the parent, destroying all its state.

Detection: Look for function declarations or arrow functions that return JSX and are defined inside another component's body (not at module level).

Bad:

const ParentComponent = () => {
  const ChildComponent = ({ name }) => {
    return <span>{name}</span>;
  };

  return <ChildComponent name="test" />;
};

Good:

const ChildComponent = ({ name }) => {
  return <span>{name}</span>;
};

const ParentComponent = () => {
  return <ChildComponent name="test" />;
};

Note: Inline render functions (not used as <Component /> but called directly like {renderItem(item)}) are acceptable.

Report format: Show the inner component name, file:line, and explain the remount problem.

Rule: direct-state-mutation

Severity: HIGH

Detect direct mutation of state objects or arrays instead of creating new references.

Detection: Look for patterns where state is modified in place:

Bad:

const addItem = (item) => {
  items.push(item);       // mutates the array
  setItems(items);        // same reference — React won't re-render
};

Good:

const addItem = (item) => {
  setItems(prev => [...prev, item]);
};

Why it matters: React uses reference equality to detect changes. Mutating the existing object and passing the same reference means React won't re-render, causing stale UI.

Report format: Show file:line, the mutation method, and suggest spread operator or functional update.

Rule: useeffect-as-handler

Severity: MEDIUM

Detect useEffect used to respond to user events instead of handling logic directly in the event handler.

Detection: Look for a pattern where:

  1. An event handler sets a piece of state (e.g., setSubmitted(true))
  2. A useEffect watches that state and performs the actual logic

Bad:

const [submitted, setSubmitted] = useState(false);

const handleSubmit = () => {
  setSubmitted(true);
};

useEffect(() => {
  if (submitted) {
    sendFormData(formData);
    setSubmitted(false);
  }
}, [submitted]);

Good:

const handleSubmit = () => {
  sendFormData(formData);
};

Why it matters: This pattern creates an unnecessary render cycle, makes the flow harder to follow, and introduces potential bugs with stale closures and race conditions.

Report format: Show the state variable acting as a trigger, the handler that sets it, and the useEffect that reacts to it. Suggest moving the logic into the handler.

Rule: hook-sharing

Severity: LOW

Detect hook patterns that are duplicated across components and could be extracted into a shared custom hook.

Detection: Look for two or more components in the same module (or in scope) that contain similar useState + useEffect combinations:

Also check if the project already has a custom hook in src/common/hooks/ or src/hooks/ that provides the same functionality being reimplemented in a component.

Bad:

// src/pages/Orders.jsx
const [loading, setLoading] = useState(false);
const [error, setError] = useState(null);
const [data, setData] = useState(null);
useEffect(() => { /* fetch /orders */ }, []);

// src/pages/Returns.jsx
const [loading, setLoading] = useState(false);
const [error, setError] = useState(null);
const [data, setData] = useState(null);
useEffect(() => { /* fetch /returns */ }, []);

Good:

// src/hooks/useFetch.js — single implementation
const useFetch = (url) => { /* loading, error, data logic */ };

// src/pages/Orders.jsx
const { loading, error, data } = useFetch('/orders');

Report format: Show the duplicated pattern, the files involved, and suggest extracting to a custom hook.

Rule: spread-props-on-dom

Severity: MEDIUM

Detect spreading unknown props directly onto native DOM elements, which can pass invalid HTML attributes to the DOM.

Detection: Look for components that receive ...props or ...rest and spread them onto a native element (<div {...props}>, <input {...rest}>). Only flag when the component does not filter the props before spreading.

Bad:

const Card = (props) => <div {...props}>{props.children}</div>;
// Caller: <Card isActive onRetry={fn} /> → "isActive" and "onRetry" end up as DOM attributes

Good:

const Card = ({ isActive, onRetry, children, ...domProps }) => {
  return <div {...domProps}>{children}</div>;
};

Report format: Show file:line, the spread expression, and suggest destructuring known props first.

Rule: no-ternary-rendering

Severity: MEDIUM

Detect conditional rendering implemented as nested ternaries inside JSX instead of early returns.

Detection: Look for components where loading states, error states, or empty states are handled as ternary expressions (? :) inside the main return block instead of as separate if blocks with their own return before the main JSX.

Bad:

const OrderList = ({ loading, error, orders }) => {
  return (
    <Container>
      {loading ? (
        <Skeleton />
      ) : error ? (
        <ErrorMessage error={error} />
      ) : orders.length === 0 ? (
        <EmptyState />
      ) : (
        orders.map(o => <OrderRow key={o.id} order={o} />)
      )}
    </Container>
  );
};

Good:

const OrderList = ({ loading, error, orders }) => {
  if (loading) {
    return <Skeleton />;
  }

  if (error) {
    return <ErrorMessage error={error} />;
  }

  if (!orders.length) {
    return <EmptyState />;
  }

  return (
    <Container>
      {orders.map(o => <OrderRow key={o.id} order={o} />)}
    </Container>
  );
};

Why it matters: Nested ternaries are hard to read, hard to extend, and hide the component's control flow. Early returns make each state explicit and keep the happy-path JSX flat.

Report format: Show file:line, the nested ternary depth, and suggest refactoring to early returns.

Severity Reference

Severity Meaning Action
HIGH Causes bugs, memory leaks, or broken rendering. Should fix before merge. Block PR
MEDIUM Maintainability or performance concern. Should fix or justify. Discuss in review
LOW Improvement opportunity. Nice to fix but not blocking. Optional

What This Skill Does NOT Cover

These are intentionally excluded — use a linter, a dedicated tool, or a separate skill: