From 41bf76aef2a7bc75068be764ec40697caad6ba39 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 31 May 2026 22:35:15 -0700 Subject: [PATCH] chore: gate high-signal react doctor rules Adds a focused React Doctor oxlint gate for high-signal state/effect rules, disables the noisy js-combine-iterations React Doctor rule, and refactors the mobile new-worktree modal away from prop-change state reset effects. --- config/oxlint-react-doctor.json | 20 +++ config/scripts/lint-react-doctor-changed.mjs | 51 +++++++ mobile/src/components/NewWorktreeModal.tsx | 143 +++++++++++-------- package.json | 4 + pnpm-lock.yaml | 55 +++++++ react-doctor.config.json | 5 + 6 files changed, 221 insertions(+), 57 deletions(-) create mode 100644 config/oxlint-react-doctor.json create mode 100644 config/scripts/lint-react-doctor-changed.mjs create mode 100644 react-doctor.config.json diff --git a/config/oxlint-react-doctor.json b/config/oxlint-react-doctor.json new file mode 100644 index 00000000000..7083b899e2f --- /dev/null +++ b/config/oxlint-react-doctor.json @@ -0,0 +1,20 @@ +{ + "$schema": "../node_modules/oxlint/configuration_schema.json", + "plugins": [], + "categories": { + "correctness": "off", + "suspicious": "off", + "pedantic": "off", + "perf": "off", + "style": "off", + "restriction": "off", + "nursery": "off" + }, + "jsPlugins": [{ "name": "react-doctor", "specifier": "oxlint-plugin-react-doctor" }], + "rules": { + "react-doctor/no-adjust-state-on-prop-change": "warn", + "react-doctor/no-derived-state-effect": "warn", + "react-doctor/no-initialize-state": "warn" + }, + "ignorePatterns": ["**/node_modules", "**/dist", "**/out"] +} diff --git a/config/scripts/lint-react-doctor-changed.mjs b/config/scripts/lint-react-doctor-changed.mjs new file mode 100644 index 00000000000..f7c22e7b6a7 --- /dev/null +++ b/config/scripts/lint-react-doctor-changed.mjs @@ -0,0 +1,51 @@ +import { existsSync } from 'node:fs' +import { spawnSync } from 'node:child_process' + +const SOURCE_FILE_PATTERN = /\.(?:[cm]?[jt]sx?)$/ + +function run(command, args) { + const result = spawnSync(command, args, { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'inherit'] + }) + if (result.error) { + throw result.error + } + if (result.status !== 0) { + return [] + } + return result.stdout.split(/\r?\n/).filter(Boolean) +} + +const changedFiles = new Set([ + ...run('git', ['diff', '--name-only', '--diff-filter=ACMRTUB']), + ...run('git', ['diff', '--cached', '--name-only', '--diff-filter=ACMRTUB']), + ...run('git', ['ls-files', '--others', '--exclude-standard']) +]) + +const lintTargets = [...changedFiles].filter( + (file) => SOURCE_FILE_PATTERN.test(file) && existsSync(file) +) + +if (lintTargets.length === 0) { + process.exit(0) +} + +const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm' +const result = spawnSync( + pnpm, + [ + 'exec', + 'oxlint', + '--config', + 'config/oxlint-react-doctor.json', + '--deny-warnings', + ...lintTargets + ], + { stdio: 'inherit' } +) + +if (result.error) { + throw result.error +} +process.exit(result.status ?? 1) diff --git a/mobile/src/components/NewWorktreeModal.tsx b/mobile/src/components/NewWorktreeModal.tsx index f307d2432d2..66030ea1fb9 100644 --- a/mobile/src/components/NewWorktreeModal.tsx +++ b/mobile/src/components/NewWorktreeModal.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react' +import { useState, useEffect, useRef } from 'react' import { View, Text, @@ -63,6 +63,19 @@ type RepoHooksResponse = { setupTrust?: SetupHookTrust } +type SetupHookDetails = { + repoId: string + command: string | null + source: string | null + trust: SetupHookTrust | null + runPolicy: SetupRunPolicy +} + +type DetectedAgentIdsState = { + connectionId: string | null + ids: Set +} + type CreateOptions = { setupOverride?: Exclude approvedSetupContentHash?: string @@ -167,26 +180,58 @@ export function NewWorktreeModal({ existingWorktreePaths, onCreated, onClose +}: Props) { + const openEpochRef = useRef(0) + const wasVisibleRef = useRef(false) + const clientEpochRef = useRef({ client, epoch: 0 }) + + // Why: each drawer opening is a fresh form session; remounting resets local + // form state before paint instead of clearing it in a visible-prop Effect. + if (visible && !wasVisibleRef.current) { + openEpochRef.current += 1 + } + wasVisibleRef.current = visible + if (clientEpochRef.current.client !== client) { + clientEpochRef.current = { client, epoch: clientEpochRef.current.epoch + 1 } + } + + return ( + + ) +} + +function NewWorktreeModalContent({ + visible, + client, + existingWorktreePaths, + onCreated, + onClose }: Props) { const [repos, setRepos] = useState([]) const [selectedRepo, setSelectedRepo] = useState(null) const [showRepoPicker, setShowRepoPicker] = useState(false) const [selectedAgentState, setSelectedAgent] = useState(AGENT_OPTIONS[0]!) const [runtimeSettings, setRuntimeSettings] = useState(null) - const [detectedAgentIds, setDetectedAgentIds] = useState | null>(null) + const [detectedAgentIdsState, setDetectedAgentIdsState] = useState( + null + ) const [agentOverriddenState, setAgentOverridden] = useState(false) const [showAgentPicker, setShowAgentPicker] = useState(false) const [sshState, setSshState] = useState(null) - const [sshConnecting, setSshConnecting] = useState(false) + const [sshConnectingTargetId, setSshConnectingTargetId] = useState(null) const [name, setName] = useState('') const [note, setNote] = useState('') const [showAdvanced, setShowAdvanced] = useState(false) - const [setupCommand, setSetupCommand] = useState(null) - const [setupSource, setSetupSource] = useState(null) - const [setupTrust, setSetupTrust] = useState(null) + const [setupHookDetails, setSetupHookDetails] = useState(null) const [trustedOrcaHooks, setTrustedOrcaHooks] = useState({}) const [setupTrustPrompt, setSetupTrustPrompt] = useState(null) - const [setupRunPolicy, setSetupRunPolicy] = useState('run-by-default') const [setupDecisionChoice, setSetupDecisionChoice] = useState { - if (!visible) { - setShowRepoPicker(false) - setShowAgentPicker(false) - return - } - if (!client) { + if (!visible || !client) { return } let stale = false - setName('') - setNote('') - setShowAdvanced(false) - setSetupCommand(null) - setSetupSource(null) - setSetupTrust(null) - setTrustedOrcaHooks({}) - setSetupTrustPrompt(null) - setSetupRunPolicy('run-by-default') - setSetupDecisionChoice(null) - setRunSetup(true) - setError('') - setCreating(false) - setShowRepoPicker(false) - setShowAgentPicker(false) - setRuntimeSettings(null) - setDetectedAgentIds(null) - setAgentOverridden(false) - setSshState(null) - setSshConnecting(false) - setSelectedAgent(AGENT_OPTIONS[0]!) - setLoading(true) void (async () => { try { @@ -305,8 +334,6 @@ export function NewWorktreeModal({ useEffect(() => { if (!visible || !client || !selectedRepoConnectionId) { - setSshState(null) - setSshConnecting(false) return } let stale = false @@ -349,11 +376,9 @@ export function NewWorktreeModal({ return } if (selectedRepoConnectionId && sshGate.status !== 'connected') { - setDetectedAgentIds(null) return } let stale = false - setDetectedAgentIds(null) void (async () => { try { const response = selectedRepoConnectionId @@ -364,12 +389,13 @@ export function NewWorktreeModal({ if (stale) { return } - setDetectedAgentIds( - response.ok ? new Set((response as RpcSuccess).result as string[]) : new Set() - ) + setDetectedAgentIdsState({ + connectionId: selectedRepoConnectionId, + ids: response.ok ? new Set((response as RpcSuccess).result as string[]) : new Set() + }) } catch { if (!stale) { - setDetectedAgentIds(new Set()) + setDetectedAgentIdsState({ connectionId: selectedRepoConnectionId, ids: new Set() }) } } })() @@ -380,9 +406,6 @@ export function NewWorktreeModal({ useEffect(() => { if (!client || !selectedRepo) { - setSetupCommand(null) - setSetupSource(null) - setSetupTrust(null) return } let stale = false @@ -398,10 +421,13 @@ export function NewWorktreeModal({ const result = (response as RpcSuccess).result as RepoHooksResponse const cmd = result.hooks?.scripts?.setup?.trim() || null const policy = result.setupRunPolicy ?? 'run-by-default' - setSetupCommand(cmd) - setSetupSource(result.source) - setSetupTrust(normalizeSetupHookTrust(result.setupTrust)) - setSetupRunPolicy(policy) + setSetupHookDetails({ + repoId: selectedRepo.id, + command: cmd, + source: result.source, + trust: normalizeSetupHookTrust(result.setupTrust), + runPolicy: policy + }) setSetupDecisionChoice(null) setRunSetup(policy !== 'skip-by-default') if (cmd && policy === 'ask') { @@ -410,10 +436,13 @@ export function NewWorktreeModal({ } } catch { if (!stale) { - setSetupCommand(null) - setSetupSource(null) - setSetupTrust(null) - setSetupRunPolicy('run-by-default') + setSetupHookDetails({ + repoId: selectedRepo.id, + command: null, + source: null, + trust: null, + runPolicy: 'run-by-default' + }) setSetupDecisionChoice(null) } } @@ -427,7 +456,7 @@ export function NewWorktreeModal({ if (!client || !selectedRepoConnectionId) { return } - setSshConnecting(true) + setSshConnectingTargetId(selectedRepoConnectionId) setSshState({ targetId: selectedRepoConnectionId, status: 'connecting', @@ -460,7 +489,7 @@ export function NewWorktreeModal({ reconnectAttempt: 0 }) } finally { - setSshConnecting(false) + setSshConnectingTargetId((current) => (current === selectedRepoConnectionId ? null : current)) } } diff --git a/package.json b/package.json index c6c4b78b41b..b8954132f2c 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,8 @@ "scripts": { "format": "oxfmt --write .", "lint": "oxlint && pnpm run lint:switch-exhaustiveness && node config/scripts/check-styled-scrollbars.mjs", + "lint:react-doctor": "oxlint --config config/oxlint-react-doctor.json", + "lint:react-doctor:changed": "node config/scripts/lint-react-doctor-changed.mjs", "lint:switch-exhaustiveness": "oxlint --type-aware --config config/oxlint-switch-exhaustiveness.json src/main src/preload src/shared src/relay src/cli src/renderer/src config tests --quiet", "prepare": "husky", "test": "node config/scripts/ensure-native-runtime.mjs --runtime=node && vitest run --config config/vitest.config.ts", @@ -139,6 +141,7 @@ "monaco-editor": "^0.55.1", "oxfmt": "^0.52.0", "oxlint": "^1.67.0", + "oxlint-plugin-react-doctor": "0.2.10", "oxlint-tsgolint": "0.23.0", "pdfjs-dist": "^5.7.284", "radix-ui": "^1.4.3", @@ -178,6 +181,7 @@ "lint-staged": { "*.{ts,tsx,js,jsx,mts,cts}": [ "oxlint", + "oxlint --config config/oxlint-react-doctor.json --deny-warnings", "oxfmt --write" ], "*.{json,css}": [ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3de3643bdf4..ff31d1a4dc2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -245,6 +245,9 @@ importers: oxlint: specifier: ^1.67.0 version: 1.67.0(oxlint-tsgolint@0.23.0) + oxlint-plugin-react-doctor: + specifier: 0.2.10 + version: 0.2.10 oxlint-tsgolint: specifier: 0.23.0 version: 0.23.0 @@ -2862,6 +2865,9 @@ packages: '@types/deep-eql@4.0.2': resolution: {integrity: sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw==} + '@types/esrecurse@4.3.1': + resolution: {integrity: sha512-xJBAbDifo5hpffDBuHl0Y8ywswbiAp/Wi7Y/GtAgSlZyIABppyurxVueOPE8LUQOxdlgi6Zqce7uoEpqNTeiUw==} + '@types/estree-jsx@1.0.5': resolution: {integrity: sha512-52CcUVNFyfb1A2ALocQw/Dd1BQFNmSdkuC3BkZ6iqhdMfQz7JWOFRuJFloOzjk+6WijU56m9oKXFAXc7o3Towg==} @@ -2954,6 +2960,10 @@ packages: '@types/yauzl@2.10.3': resolution: {integrity: sha512-oJoftv0LSuaDZE3Le4DbKX+KS9G36NzOeSap90UIK0yMA/NhKJhqlSGtNDORNRaIbQfzjXDrQa0ytJ6mNRGz/Q==} + '@typescript-eslint/types@8.60.0': + resolution: {integrity: sha512-AsE7x2XaAK+CVbeih0Fvbn+r1qHxtpLDJ3XUuFcIinT318T90yHMJC+Zgv+jUuDjQQd06HKwxnDu6sz1IcTilA==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + '@typescript/native-preview-darwin-arm64@7.0.0-dev.20260421.2': resolution: {integrity: sha512-fHv1r3ZmVo6zxuAIFmuX3w9QxbcauoG0SsWhmDwm6VmRubLlOJIcmTtlmV3JAb9oOnq8LuzZljzT7Q39fSMQDw==} cpu: [arm64] @@ -3952,11 +3962,27 @@ packages: resolution: {integrity: sha512-/veY75JbMK4j1yjvuUxuVsiS/hr/4iHs9FTT6cgTexxdE0Ly/glccBAkloH/DofkjRbZU3bnoj38mOmhkZ0lHw==} engines: {node: '>=12'} + eslint-scope@9.1.2: + resolution: {integrity: sha512-xS90H51cKw0jltxmvmHy2Iai1LIqrfbw57b79w/J7MfvDfkIkFZ+kj6zC3BjtUwh150HsSSdxXZcsuv72miDFQ==} + engines: {node: ^20.19.0 || ^22.13.0 || >=24} + + eslint-visitor-keys@5.0.1: + resolution: {integrity: sha512-tD40eHxA35h0PEIZNeIjkHoDR4YjjJp34biM0mDvplBe//mB+IHCqHDGV7pxF+7MklTvighcCPPZC7ynWyjdTA==} + engines: {node: ^20.19.0 || ^22.13.0 || >=24} + esprima@4.0.1: resolution: {integrity: sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==} engines: {node: '>=4'} hasBin: true + esrecurse@4.3.0: + resolution: {integrity: sha512-KmfKL3b6G+RXvP8N1vr3Tq1kL/oCFgn2NYXEtqP8/L3pKapUA4G8cFVaoF3SU323CD4XypR/ffioHmkti6/Tag==} + engines: {node: '>=4.0'} + + estraverse@5.3.0: + resolution: {integrity: sha512-MMdARuVEQziNTeJD8DgMqmhwR11BRQ/cBP+pLtYdSTnf3MIO8fFeiINEbX36ZdNlfU/7A9f3gUw49B3oQsvwBA==} + engines: {node: '>=4.0'} + estree-util-is-identifier-name@3.0.0: resolution: {integrity: sha512-hFtqIDZTIUZ9BXLb8y4pYGyk6+wekIivNVTcmvk8NoOh+VeRn5y6cEHzbURrWbfp1fIqdVipilzj+lfaadNZmg==} @@ -5150,6 +5176,10 @@ packages: vite-plus: optional: true + oxlint-plugin-react-doctor@0.2.10: + resolution: {integrity: sha512-n36QdOLz4k9EEWod+vhki9/h29x/PL4nS91nWSk6AIr7HAL+rc6fkwUcVLgg3NhUVlaL/VOfYiIm4cVxyIIdGg==} + engines: {node: ^20.19.0 || >=22.12.0} + oxlint-tsgolint@0.23.0: resolution: {integrity: sha512-3mBv3CoPbh8dFbzfDGIWa2ytZjn2v+3EX4aKRXjIhsoGFzG8GCjfRirz3rwZf1wYbZzsNLTSgpw8VjQuWdp/jA==} hasBin: true @@ -8702,6 +8732,8 @@ snapshots: '@types/deep-eql@4.0.2': {} + '@types/esrecurse@4.3.1': {} + '@types/estree-jsx@1.0.5': dependencies: '@types/estree': 1.0.8 @@ -8801,6 +8833,8 @@ snapshots: '@types/node': 25.6.0 optional: true + '@typescript-eslint/types@8.60.0': {} + '@typescript/native-preview-darwin-arm64@7.0.0-dev.20260421.2': optional: true @@ -9902,8 +9936,23 @@ snapshots: escape-string-regexp@5.0.0: {} + eslint-scope@9.1.2: + dependencies: + '@types/esrecurse': 4.3.1 + '@types/estree': 1.0.8 + esrecurse: 4.3.0 + estraverse: 5.3.0 + + eslint-visitor-keys@5.0.1: {} + esprima@4.0.1: {} + esrecurse@4.3.0: + dependencies: + estraverse: 5.3.0 + + estraverse@5.3.0: {} + estree-util-is-identifier-name@3.0.0: {} estree-walker@3.0.3: @@ -11422,6 +11471,12 @@ snapshots: '@oxfmt/binding-win32-ia32-msvc': 0.52.0 '@oxfmt/binding-win32-x64-msvc': 0.52.0 + oxlint-plugin-react-doctor@0.2.10: + dependencies: + '@typescript-eslint/types': 8.60.0 + eslint-scope: 9.1.2 + eslint-visitor-keys: 5.0.1 + oxlint-tsgolint@0.23.0: optionalDependencies: '@oxlint-tsgolint/darwin-arm64': 0.23.0 diff --git a/react-doctor.config.json b/react-doctor.config.json new file mode 100644 index 00000000000..6ced54a1369 --- /dev/null +++ b/react-doctor.config.json @@ -0,0 +1,5 @@ +{ + "rules": { + "react-doctor/js-combine-iterations": "off" + } +}