Skip to content

proposal: no-focus-on-mount#315

Open
NisargIO wants to merge 1 commit into
mainfrom
discovery/propose-no-focus-on-mount
Open

proposal: no-focus-on-mount#315
NisargIO wants to merge 1 commit into
mainfrom
discovery/propose-no-focus-on-mount

Conversation

@NisargIO
Copy link
Copy Markdown
Member

@NisargIO NisargIO commented May 22, 2026

Proposal: react-doctor/no-focus-on-mount

Status: 🟡 Auto-discovered draft proposal. Not yet implemented. Maintainer review wanted before any code lands.

Category state-and-effects
Severity warn
Source clusters NEW::no-focus-on-mount
Independent draft proposals 1
Backing evidence units 1

Sources

Discovered by the react-doctor-evals discovery flywheel mining bug-fix evidence across React OSS repos. The pipeline below produced this proposal:

OSS repo → Vercel Sandbox miner → EvidenceUnit → RuleDrafter (LLM) → RuleDedupe → THIS PR

Backing evidence

Validation prompt

FP-aware guidance for the react-review agent when triaging this rule:

Only treat this as a bug when the focus runs during initial mount or from an empty-deps effect. Common false positives are modal/dialog initial-focus patterns, restoring focus after navigation, and effects gated by an explicit isOpen or ready flag. If the focus is triggered by a user action or a later dependency change, it is probably intentional.

Fix prompt

Actionable fix suggestion surfaced to the user when the rule fires:

Move the focus into the user action that opens the UI, or gate it on explicit readiness instead of mounting it immediately. For example:

useEffect(() => {
  if (isOpen) inputRef.current?.focus();
}, [isOpen]);

Positive fixture (SHOULD trigger)

import { useEffect, useRef } from 'react';

export function SearchBox() {
  const inputRef = useRef<HTMLInputElement>(null);

  useEffect(() => {
    inputRef.current?.focus();
  }, []);

  return <input ref={inputRef} />;
}

Negative fixture (should NOT trigger)

import { useEffect, useRef } from 'react';

export function SearchBox({ isOpen }: { isOpen: boolean }) {
  const inputRef = useRef<HTMLInputElement>(null);

  useEffect(() => {
    if (isOpen) inputRef.current?.focus();
  }, [isOpen]);

  return <input ref={inputRef} />;
}

Proposed AST detector

Would land at packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-focus-on-mount.ts:

import { EFFECT_HOOK_NAMES } from '../../constants/react.js';
import { defineRule } from '../../utils/define-rule.js';
import type { EsTreeNode } from '../../utils/es-tree-node.js';
import type { EsTreeNodeOfType } from '../../utils/es-tree-node-of-type.js';
import { getEffectCallback } from '../../utils/get-effect-callback.js';
import { isHookCall } from '../../utils/is-hook-call.js';
import { isNodeOfType } from '../../utils/is-node-of-type.js';
import type { Rule } from '../../utils/rule.js';
import type { RuleContext } from '../../utils/rule-context.js';
import { walkAst } from '../../utils/walk-ast.js';

const unwrapChainExpression = (node: EsTreeNode): EsTreeNode =>
  isNodeOfType(node, 'ChainExpression') ? node.expression : node;

const hasEmptyDependencyArray = (node: EsTreeNode): boolean => {
  const depsNode = node.arguments?.[1];
  return isNodeOfType(depsNode, 'ArrayExpression') && (depsNode.elements?.length ?? 0) === 0;
};

const isFocusCall = (node: EsTreeNode): boolean => {
  if (!isNodeOfType(node, 'CallExpression')) return false;
  const callee = unwrapChainExpression(node.callee);
  if (!isNodeOfType(callee, 'MemberExpression')) return false;
  if (callee.computed) return false;
  if (!isNodeOfType(callee.property, 'Identifier')) return false;
  return callee.property.name === 'focus';
};

const effectCallbackContainsFocusCall = (callback: EsTreeNode): boolean => {
  let didFindFocusCall = false;
  walkAst(callback, (child: EsTreeNode) => {
    if (didFindFocusCall) return false;
    if (isFocusCall(child)) didFindFocusCall = true;
  });
  return didFindFocusCall;
};

export const noFocusOnMount = defineRule<Rule>({
  id: 'no-focus-on-mount',
  severity: 'warn',
  recommendation:
    'Move the focus into the user action that opens the UI, or gate it on an explicit ready/open state instead of running it on mount.\n\n```tsx\nuseEffect(() => {\n  if (isOpen) inputRef.current?.focus();\n}, [isOpen]);\n```',
  create: (context: RuleContext) => ({
    CallExpression(node: EsTreeNodeOfType<'CallExpression'>) {
      if (!isHookCall(node, EFFECT_HOOK_NAMES)) return;
      if (!hasEmptyDependencyArray(node)) return;
      const callback = getEffectCallback(node);
      if (!callback) return;
      if (!effectCallbackContainsFocusCall(callback)) return;
      context.report({
        node,
        message:
          'focus() in a mount effect can steal focus before the UI is ready - move it behind a user action or an explicit open state',
      });
    },
  }),
});

Generated by `rde discover` (see [millionco/react-doctor-evals#11](https://github.com/millionco/react-doctor-evals/pull/11) for the pipeline). Implementation, test fixtures, and rule registration are deliberately deferred — this PR exists for maintainer triage of the proposal only. Reject, edit-and-approve, or merge after wiring as you see fit.

Note

Low Risk
Adds a markdown-only draft proposal with no runtime or build impact. Risk is limited to potential reviewer confusion/maintenance overhead if proposals are treated as implemented rules.

Overview
Adds a new markdown proposal, proposals/no-focus-on-mount.md, describing a not-yet-implemented react-doctor/no-focus-on-mount rule that would warn when focus() is called inside an empty-deps effect on mount.

The document includes false-positive guidance, a suggested fix pattern, example positive/negative fixtures, and a proposed AST detector outline, but no code is wired into the plugin in this PR.

Reviewed by Cursor Bugbot for commit e687437. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
react-doctor-website Ready Ready Preview, Comment May 22, 2026 8:56am

@reactreview
Copy link
Copy Markdown

reactreview Bot commented May 22, 2026

No new issues

Reviewed by reactreview for commit e687437. Configure here.

Auto-discovered draft proposal. No implementation yet — see file body for the proposed detector + prompts + evidence. See millionco/react-doctor-evals#11 for the discovery pipeline.
@NisargIO NisargIO force-pushed the discovery/propose-no-focus-on-mount branch from 0fb05e6 to e687437 Compare May 22, 2026 08:56
@aidenybai aidenybai marked this pull request as ready for review May 22, 2026 09:01
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e687437. Configure here.

if (!hasEmptyDependencyArray(node)) return;
const callback = getEffectCallback(node);
if (!callback) return;
if (!effectCallbackContainsFocusCall(callback)) return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detector flags non-mount focus

Medium Severity

The proposed effectCallbackContainsFocusCall walk treats any .focus() under the effect callback as mount-time focus, including calls inside if (isOpen) guards, nested handlers, and cleanup returns. That conflicts with the proposal’s own FP guidance for gated, modal, and user-triggered focus.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e687437. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant