diff --git a/.changeset/port-effect-rules.md b/.changeset/port-effect-rules.md new file mode 100644 index 000000000..7fe09b7eb --- /dev/null +++ b/.changeset/port-effect-rules.md @@ -0,0 +1,38 @@ +--- +"@react-doctor/core": patch +"oxlint-plugin-react-doctor": patch +"react-doctor": patch +--- + +Natively port the 8 rules from `eslint-plugin-react-you-might-not-need-an-effect` +(NickvanDyke, MIT) into `oxlint-plugin-react-doctor`. They now ship as +`react-doctor/*` rules and no longer require the optional peer +dependency. The optional peer-dep surface (`effect/*` rules, +`resolveYouMightNotNeedEffectPlugin`, +`YOU_MIGHT_NOT_NEED_EFFECT_NAMESPACE`) is removed from +`@react-doctor/core`. + +The ports use a real `eslint-scope` ScopeManager (cached per Program +via `WeakMap`) — same `references` / `resolved.defs[].node.init` / +`isEventualCallTo` chasing the upstream plugin uses. Diagnostic +messages match upstream verbatim with template variables substituted +in JS. + +| Rule (now `react-doctor/`) | What it catches | +| ----------------------------------- | ------------------------------------------------------------------------ | +| `no-derived-state` | Storing derived state via a useEffect instead of computing during render | +| `no-chain-state-updates` | Chaining state updates across effects | +| `no-event-handler` | Using state + a guarded effect as an event handler | +| `no-adjust-state-on-prop-change` | Adjusting state in an effect when a prop changes | +| `no-reset-all-state-on-prop-change` | Resetting all state in an effect (use a `key` prop) | +| `no-pass-live-state-to-parent` | Pushing live state to a parent via a callback in an effect | +| `no-pass-data-to-parent` | Passing fetched data to a parent via a callback in an effect | +| `no-initialize-state` | Initializing state inside a mount-only effect | + +Parity coverage: 147 of 148 upstream test cases pass (the 1 remaining +case is upstream's own `todo: true`, a renamed-`useState` import). + +These coexist with React Doctor's existing thematically-related rules +(`no-derived-state-effect`, `no-effect-chain`, `no-event-trigger-state`, +`no-prop-callback-in-effect`) — different IDs, different shapes, +different messages. diff --git a/packages/core/package.json b/packages/core/package.json index 853f48989..1043b936a 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -25,19 +25,14 @@ }, "devDependencies": { "@types/node": "^25.6.0", - "eslint-plugin-react-hooks": "^7.1.1", - "eslint-plugin-react-you-might-not-need-an-effect": "^0.10.1" + "eslint-plugin-react-hooks": "^7.1.1" }, "peerDependencies": { - "eslint-plugin-react-hooks": "^6 || ^7", - "eslint-plugin-react-you-might-not-need-an-effect": "^0.10" + "eslint-plugin-react-hooks": "^6 || ^7" }, "peerDependenciesMeta": { "eslint-plugin-react-hooks": { "optional": true - }, - "eslint-plugin-react-you-might-not-need-an-effect": { - "optional": true } }, "engines": { diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index 2ed13f0a2..08ee97aa0 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -33,12 +33,14 @@ export const FETCH_TIMEOUT_MS = 10_000; export const SPAWN_ARGS_MAX_LENGTH_CHARS = 24_000; // HACK: bound per-batch work so that JS-evaluated plugins with bad -// scaling (notably `eslint-plugin-react-you-might-not-need-an-effect` -// — verified to hit the 5-min spawn timeout on supabase/studio's ~3500 -// source files at batch=500, productive at batch=100) stay tractable -// AND so that oxlint doesn't SIGABRT from memory pressure on very -// large file sets. Smaller batches add ~50ms spawn overhead per -// extra batch — negligible vs the hard-cap perf cliffs they prevent. +// scaling (originally the upstream `effect` plugin — verified to hit +// the 5-min spawn timeout on supabase/studio's ~3500 source files at +// batch=500, productive at batch=100; same characteristics apply to +// the ported `react-doctor/no-derived-state` family because both rely +// on whole-component scope walking) stay tractable AND so that oxlint +// doesn't SIGABRT from memory pressure on very large file sets. +// Smaller batches add ~50ms spawn overhead per extra batch — negligible +// vs the hard-cap perf cliffs they prevent. export const OXLINT_MAX_FILES_PER_BATCH = 100; export const DEFAULT_BRANCH_CANDIDATES = ["main", "master"]; diff --git a/packages/core/src/run-oxlint.ts b/packages/core/src/run-oxlint.ts index 2864d7aa0..f1888496f 100644 --- a/packages/core/src/run-oxlint.ts +++ b/packages/core/src/run-oxlint.ts @@ -497,13 +497,14 @@ export const runOxlint = async (options: RunOxlintOptions): Promise => { diff --git a/packages/core/src/runners/oxlint/config.ts b/packages/core/src/runners/oxlint/config.ts index 518123ec2..e8b658e61 100644 --- a/packages/core/src/runners/oxlint/config.ts +++ b/packages/core/src/runners/oxlint/config.ts @@ -3,17 +3,11 @@ import reactDoctorPlugin, { BUILTIN_A11Y_RULES, BUILTIN_REACT_RULES, REACT_COMPILER_RULES, - YOU_MIGHT_NOT_NEED_EFFECT_RULES, } from "oxlint-plugin-react-doctor"; import type { OxlintRuleSeverity } from "oxlint-plugin-react-doctor"; import type { ProjectInfo } from "@react-doctor/types"; import { buildCapabilities, shouldEnableRule } from "./capabilities.js"; -import { - filterRulesToAvailable, - resolveReactHooksJsPlugin, - resolveYouMightNotNeedEffectPlugin, - YOU_MIGHT_NOT_NEED_EFFECT_NAMESPACE, -} from "./plugin-resolution.js"; +import { filterRulesToAvailable, resolveReactHooksJsPlugin } from "./plugin-resolution.js"; import type { JsPluginEntry } from "./plugin-resolution.js"; export interface OxlintConfigOptions { @@ -47,18 +41,8 @@ export const createOxlintConfig = ({ ) : {}; - const youMightNotNeedEffectPlugin = resolveYouMightNotNeedEffectPlugin(customRulesOnly); - const youMightNotNeedEffectRules = youMightNotNeedEffectPlugin - ? filterRulesToAvailable( - YOU_MIGHT_NOT_NEED_EFFECT_RULES, - YOU_MIGHT_NOT_NEED_EFFECT_NAMESPACE, - youMightNotNeedEffectPlugin.availableRuleNames, - ) - : {}; - const jsPlugins: JsPluginEntry[] = []; if (reactHooksJsPlugin) jsPlugins.push(reactHooksJsPlugin.entry); - if (youMightNotNeedEffectPlugin) jsPlugins.push(youMightNotNeedEffectPlugin.entry); const capabilities = buildCapabilities(project); @@ -99,7 +83,6 @@ export const createOxlintConfig = ({ ...(customRulesOnly ? {} : BUILTIN_REACT_RULES), ...(customRulesOnly ? {} : BUILTIN_A11Y_RULES), ...reactCompilerRules, - ...youMightNotNeedEffectRules, ...enabledReactDoctorRules, }, }; diff --git a/packages/core/src/runners/oxlint/plugin-resolution.ts b/packages/core/src/runners/oxlint/plugin-resolution.ts index 52b2db59a..f4105ceb3 100644 --- a/packages/core/src/runners/oxlint/plugin-resolution.ts +++ b/packages/core/src/runners/oxlint/plugin-resolution.ts @@ -16,11 +16,6 @@ interface ResolvedReactHooksJsPlugin { availableRuleNames: ReadonlySet; } -interface ResolvedYouMightNotNeedEffectPlugin { - entry: JsPluginEntry; - availableRuleNames: ReadonlySet; -} - interface MaybePluginModule { rules?: Record; default?: { rules?: Record }; @@ -62,29 +57,6 @@ export const resolveReactHooksJsPlugin = ( }; }; -// HACK: oxlint-namespaces this third-party ESLint plugin under -// `effect` so the long upstream package name doesn't clutter rule -// keys. Issue #187 - adds the plugin's complementary rule surface -// alongside react-doctor's native `state-and-effects` rules. The -// plugin is opt-in: skipped when not installed (peer is optional). -export const YOU_MIGHT_NOT_NEED_EFFECT_NAMESPACE = "effect"; - -export const resolveYouMightNotNeedEffectPlugin = ( - customRulesOnly: boolean, -): ResolvedYouMightNotNeedEffectPlugin | null => { - if (customRulesOnly) return null; - let pluginSpecifier: string; - try { - pluginSpecifier = esmRequire.resolve("eslint-plugin-react-you-might-not-need-an-effect"); - } catch { - return null; - } - return { - entry: { name: YOU_MIGHT_NOT_NEED_EFFECT_NAMESPACE, specifier: pluginSpecifier }, - availableRuleNames: readPluginRuleNames(pluginSpecifier), - }; -}; - export const filterRulesToAvailable = ( rules: Record, pluginNamespace: string, diff --git a/packages/oxlint-plugin-react-doctor/README.md b/packages/oxlint-plugin-react-doctor/README.md index bfe8d84a2..0feac456b 100644 --- a/packages/oxlint-plugin-react-doctor/README.md +++ b/packages/oxlint-plugin-react-doctor/README.md @@ -5,7 +5,7 @@ [oxlint](https://oxc.rs/docs/guide/usage/linter) plugin for [React Doctor](https://react.doctor). Diagnoses React codebases for security, performance, correctness, accessibility, bundle-size, and architecture issues. -This package owns the rule implementations (178 rules across architecture, performance, correctness, security, accessibility, bundle-size, and framework-specific buckets). [`eslint-plugin-react-doctor`](https://npmjs.com/package/eslint-plugin-react-doctor) wraps these same rules for ESLint, and the full diagnostic CLI lives in [`react-doctor`](https://npmjs.com/package/react-doctor). +This package owns the rule implementations (187 rules across architecture, performance, correctness, security, accessibility, bundle-size, and framework-specific buckets). [`eslint-plugin-react-doctor`](https://npmjs.com/package/eslint-plugin-react-doctor) wraps these same rules for ESLint, and the full diagnostic CLI lives in [`react-doctor`](https://npmjs.com/package/react-doctor). ## Install @@ -56,6 +56,23 @@ Each rule can be set to `"error"`, `"warn"`, or `"off"`: } ``` +## "You Might Not Need an Effect" rule family + +Eight rules ported 1:1 from [`eslint-plugin-react-you-might-not-need-an-effect`](https://github.com/NickvanDyke/eslint-plugin-react-you-might-not-need-an-effect) (MIT, NickvanDyke) ship natively in this package — same rule IDs, same diagnostic messages, same semantics (147 of 148 upstream test cases pass; the remaining one is upstream's own `todo: true`). Attribution and known divergences live in [`SOURCE.md`](https://github.com/millionco/react-doctor/blob/main/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect/SOURCE.md). + +| Rule | What it catches | +| ------------------------------------------------ | ----------------------------------------------------------------------------- | +| `react-doctor/no-derived-state` | Storing derived state via `useEffect` instead of computing during render | +| `react-doctor/no-chain-state-updates` | Chaining state updates across effects | +| `react-doctor/no-event-handler` | Using state + a guarded effect as an event handler | +| `react-doctor/no-adjust-state-on-prop-change` | Adjusting state in an effect when a prop changes | +| `react-doctor/no-reset-all-state-on-prop-change` | Resetting all state in an effect (use a `key` prop instead) | +| `react-doctor/no-pass-live-state-to-parent` | Pushing live state to a parent via a callback in an effect | +| `react-doctor/no-pass-data-to-parent` | Passing fetched data to a parent via a callback in an effect | +| `react-doctor/no-initialize-state` | Initializing state inside a mount-only effect (pass it to `useState` instead) | + +If you previously enabled them as `effect/*` via the optional peer dep, drop the peer dep — they're enabled by default through React Doctor's CLI config now. + ## Want the CLI too? This package only ships the oxlint plugin. To run React Doctor's full scan (with scoring, JSON reports, agent integration, etc.), use the main CLI: diff --git a/packages/oxlint-plugin-react-doctor/package.json b/packages/oxlint-plugin-react-doctor/package.json index dfac9a4d2..8729b4a7d 100644 --- a/packages/oxlint-plugin-react-doctor/package.json +++ b/packages/oxlint-plugin-react-doctor/package.json @@ -49,7 +49,11 @@ "test": "pnpm gen && vp test run" }, "dependencies": { - "@typescript-eslint/types": "^8.59.3" + "@types/eslint-scope": "^9.1.0", + "@types/eslint-visitor-keys": "^3.3.2", + "@typescript-eslint/types": "^8.59.3", + "eslint-scope": "^9.1.2", + "eslint-visitor-keys": "^5.0.1" }, "devDependencies": { "@react-doctor/types": "workspace:*", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts index c6995dba3..4807e5a13 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts @@ -52,11 +52,14 @@ import { nextjsNoPolyfillScript } from "./rules/nextjs/nextjs-no-polyfill-script import { nextjsNoRedirectInTryCatch } from "./rules/nextjs/nextjs-no-redirect-in-try-catch.js"; import { nextjsNoSideEffectInGetHandler } from "./rules/nextjs/nextjs-no-side-effect-in-get-handler.js"; import { nextjsNoUseSearchParamsWithoutSuspense } from "./rules/nextjs/nextjs-no-use-search-params-without-suspense.js"; +import { noAdjustStateOnPropChange } from "./rules/state-and-effects/no-adjust-state-on-prop-change.js"; import { noArrayIndexAsKey } from "./rules/correctness/no-array-index-as-key.js"; import { noBarrelImport } from "./rules/bundle-size/no-barrel-import.js"; import { noCascadingSetState } from "./rules/state-and-effects/no-cascading-set-state.js"; +import { noChainStateUpdates } from "./rules/state-and-effects/no-chain-state-updates.js"; import { noDarkModeGlow } from "./rules/design/no-dark-mode-glow.js"; import { noDefaultProps } from "./rules/architecture/no-default-props.js"; +import { noDerivedState } from "./rules/state-and-effects/no-derived-state.js"; import { noDerivedStateEffect } from "./rules/state-and-effects/no-derived-state-effect.js"; import { noDerivedUseState } from "./rules/state-and-effects/no-derived-use-state.js"; import { noDirectStateMutation } from "./rules/state-and-effects/no-direct-state-mutation.js"; @@ -67,6 +70,7 @@ import { noEffectChain } from "./rules/state-and-effects/no-effect-chain.js"; import { noEffectEventHandler } from "./rules/state-and-effects/no-effect-event-handler.js"; import { noEffectEventInDeps } from "./rules/state-and-effects/no-effect-event-in-deps.js"; import { noEval } from "./rules/security/no-eval.js"; +import { noEventHandler } from "./rules/state-and-effects/no-event-handler.js"; import { noEventTriggerState } from "./rules/state-and-effects/no-event-trigger-state.js"; import { noFetchInEffect } from "./rules/state-and-effects/no-fetch-in-effect.js"; import { noFlushSync } from "./rules/view-transitions/no-flush-sync.js"; @@ -76,6 +80,7 @@ import { noGiantComponent } from "./rules/architecture/no-giant-component.js"; import { noGlobalCssVariableAnimation } from "./rules/performance/no-global-css-variable-animation.js"; import { noGradientText } from "./rules/design/no-gradient-text.js"; import { noGrayOnColoredBackground } from "./rules/design/no-gray-on-colored-background.js"; +import { noInitializeState } from "./rules/state-and-effects/no-initialize-state.js"; import { noInlineBounceEasing } from "./rules/design/no-inline-bounce-easing.js"; import { noInlineExhaustiveStyle } from "./rules/design/no-inline-exhaustive-style.js"; import { noInlinePropOnMemoComponent } from "./rules/performance/no-inline-prop-on-memo-component.js"; @@ -92,6 +97,8 @@ import { noMoment } from "./rules/bundle-size/no-moment.js"; import { noMutableInDeps } from "./rules/state-and-effects/no-mutable-in-deps.js"; import { noNestedComponentDefinition } from "./rules/architecture/no-nested-component-definition.js"; import { noOutlineNone } from "./rules/design/no-outline-none.js"; +import { noPassDataToParent } from "./rules/state-and-effects/no-pass-data-to-parent.js"; +import { noPassLiveStateToParent } from "./rules/state-and-effects/no-pass-live-state-to-parent.js"; import { noPermanentWillChange } from "./rules/performance/no-permanent-will-change.js"; import { noPolymorphicChildren } from "./rules/correctness/no-polymorphic-children.js"; import { noPreventDefault } from "./rules/correctness/no-prevent-default.js"; @@ -101,6 +108,7 @@ import { noReactDomDeprecatedApis } from "./rules/architecture/no-react-dom-depr import { noReact19DeprecatedApis } from "./rules/architecture/no-react19-deprecated-apis.js"; import { noRenderInRender } from "./rules/architecture/no-render-in-render.js"; import { noRenderPropChildren } from "./rules/architecture/no-render-prop-children.js"; +import { noResetAllStateOnPropChange } from "./rules/state-and-effects/no-reset-all-state-on-prop-change.js"; import { noScaleFromZero } from "./rules/performance/no-scale-from-zero.js"; import { noSecretsInClientCode } from "./rules/security/no-secrets-in-client-code.js"; import { noSetStateInRender } from "./rules/state-and-effects/no-set-state-in-render.js"; @@ -761,6 +769,19 @@ export const reactDoctorRules = [ category: "Next.js", }, }, + { + key: "react-doctor/no-adjust-state-on-prop-change", + id: "no-adjust-state-on-prop-change", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noAdjustStateOnPropChange, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-array-index-as-key", id: "no-array-index-as-key", @@ -800,6 +821,19 @@ export const reactDoctorRules = [ category: "State & Effects", }, }, + { + key: "react-doctor/no-chain-state-updates", + id: "no-chain-state-updates", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noChainStateUpdates, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-dark-mode-glow", id: "no-dark-mode-glow", @@ -826,6 +860,19 @@ export const reactDoctorRules = [ category: "Architecture", }, }, + { + key: "react-doctor/no-derived-state", + id: "no-derived-state", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noDerivedState, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-derived-state-effect", id: "no-derived-state-effect", @@ -956,6 +1003,19 @@ export const reactDoctorRules = [ category: "Security", }, }, + { + key: "react-doctor/no-event-handler", + id: "no-event-handler", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noEventHandler, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-event-trigger-state", id: "no-event-trigger-state", @@ -1073,6 +1133,19 @@ export const reactDoctorRules = [ category: "Accessibility", }, }, + { + key: "react-doctor/no-initialize-state", + id: "no-initialize-state", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noInitializeState, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-inline-bounce-easing", id: "no-inline-bounce-easing", @@ -1281,6 +1354,32 @@ export const reactDoctorRules = [ category: "Accessibility", }, }, + { + key: "react-doctor/no-pass-data-to-parent", + id: "no-pass-data-to-parent", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noPassDataToParent, + framework: "global", + category: "State & Effects", + }, + }, + { + key: "react-doctor/no-pass-live-state-to-parent", + id: "no-pass-live-state-to-parent", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noPassLiveStateToParent, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-permanent-will-change", id: "no-permanent-will-change", @@ -1398,6 +1497,19 @@ export const reactDoctorRules = [ category: "Architecture", }, }, + { + key: "react-doctor/no-reset-all-state-on-prop-change", + id: "no-reset-all-state-on-prop-change", + source: "react-doctor", + framework: "global", + category: "State & Effects", + severity: "warn", + rule: { + ...noResetAllStateOnPropChange, + framework: "global", + category: "State & Effects", + }, + }, { key: "react-doctor/no-scale-from-zero", id: "no-scale-from-zero", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect/SOURCE.md b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect/SOURCE.md new file mode 100644 index 000000000..85e9d860e --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect/SOURCE.md @@ -0,0 +1,93 @@ +# Ported from `eslint-plugin-react-you-might-not-need-an-effect` + +Upstream: https://github.com/NickvanDyke/eslint-plugin-react-you-might-not-need-an-effect + +- Upstream package: `eslint-plugin-react-you-might-not-need-an-effect` +- Upstream commit SHA at time of port: `4c71faaa7623d2d5feb33983dc2ebcc08206bcc5` +- Upstream version at time of port: `0.10.1` (HEAD of `main`) +- Upstream license: **MIT** (Copyright © 2025 Nick van Dyke) + +The eight rules listed below are AST-only 1:1 ports of the upstream +ESLint plugin's rule set. Diagnostic messages match upstream verbatim +(with `{{state}}` / `{{arguments}}` / `{{prop}}` template variables +substituted in JS rather than via ESLint's message templating). + +| Rule ID (this package, `react-doctor/`) | Upstream file | +| ------------------------------------------- | ------------------------------------------------ | +| `no-derived-state` | `src/rules/no-derived-state.js` | +| `no-chain-state-updates` | `src/rules/no-chain-state-updates.js` | +| `no-event-handler` | `src/rules/no-event-handler.js` | +| `no-adjust-state-on-prop-change` | `src/rules/no-adjust-state-on-prop-change.js` | +| `no-reset-all-state-on-prop-change` | `src/rules/no-reset-all-state-on-prop-change.js` | +| `no-pass-live-state-to-parent` | `src/rules/no-pass-live-state-to-parent.js` | +| `no-pass-data-to-parent` | `src/rules/no-pass-data-to-parent.js` | +| `no-initialize-state` | `src/rules/no-initialize-state.js` | + +## Why this lives under `react-doctor/` + +Before this port, the upstream rules were activated as +`effect/` via `eslint-plugin-react-you-might-not-need-an-effect` +discovered at scan time by `packages/core/src/runners/oxlint/plugin-resolution.ts`. +After this port, the same rule semantics ship inside +`oxlint-plugin-react-doctor` so projects no longer need the optional +peer dependency. The pre-existing thematically-related rules +(`no-derived-state-effect`, `no-effect-chain`, `no-event-trigger-state`, +`no-prop-callback-in-effect`) remain — they target different code +shapes with different messages. + +## Impedance mismatch with the upstream + +The upstream plugin is ESLint-native and uses +`context.sourceCode.getScope().references[]` plus +`ref.resolved.defs[].node.init/body` recursively to chase the +ultimate source of every value (its "upstream refs"). Oxlint JS +plugins have no scope manager — only AST visitors. The port builds +an equivalent component-scoped binding table (see +`../utils/effect/analyze-component-bindings.ts`) that classifies +every local name as `state` / `setter` / `ref` / `prop` / `constant` +/ `local-function` / `data` and precomputes +`callsAnyStateSetter` / `callsAnyPropFunction` / `callsAnyRefMethod` +flags on local function bindings. This is the AST-only analog of +upstream's `isStateSetterCall` / `isPropCall` / `isRefCall`. + +## Known divergences + +- **Renamed `useState` imports** — `import { useState as useStateFoo } +from "react"; const [x, setX] = useStateFoo(...)`. Upstream + documents this as a `todo: true` test (skipped); we mirror the + same limitation. +- **`useState` calls inside conditionals or loops** — illegal under + the rules of hooks, but technically parseable. Upstream ignores + them; we ignore them via top-level-only collection in + `collectUseStateBindings`. +- **Diagnostic message templates** — upstream uses + `messageId: "avoidDerivedState", data: { state: "fullName" }`, + which ESLint expands via the `meta.messages` table. Oxlint plugins + emit pre-substituted strings. The substituted text matches upstream + byte-for-byte; the `messageId` is not exposed as a separate field. + +## Upstream `LICENSE` (MIT, retained for attribution) + +``` +MIT License + +Copyright (c) 2025 Nick van Dyke + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +``` diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-adjust-state-on-prop-change.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-adjust-state-on-prop-change.ts new file mode 100644 index 000000000..16b4988b7 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-adjust-state-on-prop-change.ts @@ -0,0 +1,64 @@ +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 type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { + getArgsUpstreamRefs, + getCallExpr, + getUpstreamRefs, + isSynchronous, +} from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + getEffectDepsRefs, + getEffectFn, + getEffectFnRefs, + isProp, + isStateSetterCall, + isUseEffect, +} from "./utils/effect/react.js"; + +// 1:1 port of upstream `src/rules/no-adjust-state-on-prop-change.js`. +// Note: upstream does NOT skip on cleanup return. + +export const noAdjustStateOnPropChange = defineRule({ + id: "no-adjust-state-on-prop-change", + severity: "warn", + recommendation: + "Adjust the state inline during render instead of via a useEffect, or refactor the state to avoid the need entirely. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + const depsRefs = getEffectDepsRefs(analysis, node); + if (!effectFnRefs || !depsRefs) return; + const effectFn = getEffectFn(node); + if (!effectFn) return; + + const isSomeDepsProps = depsRefs + .flatMap((ref) => getUpstreamRefs(analysis, ref)) + .some((ref) => isProp(analysis, ref)); + if (!isSomeDepsProps) return; + + for (const ref of effectFnRefs) { + if (!isStateSetterCall(analysis, ref)) continue; + if (!isSynchronous(ref.identifier as unknown as EsTreeNode, effectFn)) continue; + const callExpr = getCallExpr(ref); + if (!callExpr) continue; + // Avoid overlap with no-derived-state + const isSomeArgsProps = getArgsUpstreamRefs(analysis, ref).some((argRef) => + isProp(analysis, argRef), + ); + if (isSomeArgsProps) continue; + context.report({ + node: callExpr, + message: + "Avoid adjusting state when a prop changes. Instead, adjust the state directly during render, or refactor your state to avoid this need entirely.", + }); + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-chain-state-updates.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-chain-state-updates.ts new file mode 100644 index 000000000..d830b74af --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-chain-state-updates.ts @@ -0,0 +1,65 @@ +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 type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { + getArgsUpstreamRefs, + getCallExpr, + getUpstreamRefs, + isSynchronous, +} from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + getEffectDepsRefs, + getEffectFn, + getEffectFnRefs, + hasCleanup, + isState, + isStateSetterCall, + isUseEffect, +} from "./utils/effect/react.js"; + +// 1:1 port of upstream +// `src/rules/no-chain-state-updates.js`. + +export const noChainStateUpdates = defineRule({ + id: "no-chain-state-updates", + severity: "warn", + recommendation: + "Update all related state simultaneously inside the event handler that originally fires, instead of reacting to one state update in a useEffect that writes another state. See https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node) || hasCleanup(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + const depsRefs = getEffectDepsRefs(analysis, node); + if (!effectFnRefs || !depsRefs) return; + const effectFn = getEffectFn(node); + if (!effectFn) return; + + const isSomeDepsState = depsRefs + .flatMap((ref) => getUpstreamRefs(analysis, ref)) + .some((ref) => isState(ref)); + if (!isSomeDepsState) return; + + for (const ref of effectFnRefs) { + if (!isStateSetterCall(analysis, ref)) continue; + if (!isSynchronous(ref.identifier as unknown as EsTreeNode, effectFn)) continue; + const callExpr = getCallExpr(ref); + if (!callExpr) continue; + // Avoid overlap with no-derived-state + const isSomeArgsState = getArgsUpstreamRefs(analysis, ref).some((argRef) => + isState(argRef), + ); + if (isSomeArgsState) continue; + context.report({ + node: callExpr, + message: + "Avoid chaining state changes. When possible, update all relevant state simultaneously.", + }); + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-derived-state.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-derived-state.ts new file mode 100644 index 000000000..02f22c452 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-derived-state.ts @@ -0,0 +1,108 @@ +import type { Reference } from "eslint-scope"; +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 { isNodeOfType } from "../../utils/is-node-of-type.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { + getArgsUpstreamRefs, + getCallExpr, + getUpstreamRefs, + isSynchronous, +} from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + getEffectDepsRefs, + getEffectFn, + getEffectFnRefs, + getUseStateDecl, + hasCleanup, + isProp, + isState, + isStateSetterCall, + isUseEffect, +} from "./utils/effect/react.js"; + +// 1:1 port of upstream +// `eslint-plugin-react-you-might-not-need-an-effect/src/rules/no-derived-state.js`. +// Diagnostic messages match upstream verbatim. The ESLint scope APIs +// upstream uses (`context.sourceCode.getScope`, `ref.resolved.defs`) +// are sourced from a cached eslint-scope `ScopeManager` via +// `getProgramAnalysis(node)`. + +const countSetterCallSites = (ref: Reference): number => { + if (!ref.resolved) return 0; + let count = 0; + for (const reference of ref.resolved.references) { + const parent = (reference.identifier as unknown as { parent?: EsTreeNode | null }).parent; + if (parent && isNodeOfType(parent, "CallExpression")) count += 1; + } + return count; +}; + +const getStateNameForUseStateDecl = (useStateNode: EsTreeNode | null): string | null => { + if (!useStateNode || !isNodeOfType(useStateNode, "VariableDeclarator")) return null; + if (!isNodeOfType(useStateNode.id, "ArrayPattern")) return null; + const elements = useStateNode.id.elements ?? []; + const candidate = elements[0] ?? elements[1]; + if (!candidate) return null; + return isNodeOfType(candidate, "Identifier") ? candidate.name : null; +}; + +export const noDerivedState = defineRule({ + id: "no-derived-state", + severity: "warn", + recommendation: + "Compute derived state inline during render (or with useMemo if expensive) instead of mirroring it into useState via a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node) || hasCleanup(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + const depsRefs = getEffectDepsRefs(analysis, node); + if (!effectFnRefs || !depsRefs) return; + const effectFn = getEffectFn(node); + if (!effectFn) return; + + for (const ref of effectFnRefs) { + if (!isStateSetterCall(analysis, ref)) continue; + if (!isSynchronous(ref.identifier as unknown as EsTreeNode, effectFn)) continue; + + const callExpr = getCallExpr(ref); + if (!callExpr) continue; + const useStateNode = getUseStateDecl(analysis, ref); + const stateName = getStateNameForUseStateDecl(useStateNode) ?? ""; + + const argsUpstreamRefs = getArgsUpstreamRefs(analysis, ref); + const depsUpstreamRefs: Reference[] = depsRefs.flatMap((depRef) => + getUpstreamRefs(analysis, depRef), + ); + + const isSomeArgsInternal = argsUpstreamRefs.some( + (argRef) => isState(argRef) || isProp(analysis, argRef), + ); + + const isAllArgsInDeps = + argsUpstreamRefs.length > 0 && + argsUpstreamRefs.every((argRef) => + depsUpstreamRefs.some((depRef) => argRef.resolved === depRef.resolved), + ); + const isValueAlwaysInSync = isAllArgsInDeps && countSetterCallSites(ref) === 1; + + if (isSomeArgsInternal) { + context.report({ + node: callExpr, + message: `Avoid storing derived state. Compute "${stateName}" directly during render, optionally with \`useMemo\` if it's expensive.`, + }); + } else if (isValueAlwaysInSync) { + context.report({ + node: callExpr, + message: `Avoid storing derived state. "${stateName}" is only set here, and thus could be computed directly during render.`, + }); + } + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-event-handler.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-event-handler.ts new file mode 100644 index 000000000..7e147e983 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-event-handler.ts @@ -0,0 +1,56 @@ +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 { isNodeOfType } from "../../utils/is-node-of-type.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { findDownstreamNodes, getDownstreamRefs, getUpstreamRefs } from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { getEffectFnRefs, hasCleanup, isProp, isState, isUseEffect } from "./utils/effect/react.js"; + +// 1:1 port of upstream `src/rules/no-event-handler.js`. + +export const noEventHandler = defineRule({ + id: "no-event-handler", + severity: "warn", + recommendation: + "Move the side effect into the event handler that triggers it, instead of guarding on its state inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node) || hasCleanup(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + if (!effectFnRefs) return; + + const ifStatementsNoElse = findDownstreamNodes(node, "IfStatement").filter( + (ifNode) => isNodeOfType(ifNode, "IfStatement") && !ifNode.alternate, + ); + const ifTestRefs = ifStatementsNoElse.flatMap((ifNode) => { + if (!isNodeOfType(ifNode, "IfStatement")) return []; + return getDownstreamRefs(analysis, ifNode.test as EsTreeNode).flatMap((ref) => + getUpstreamRefs(analysis, ref), + ); + }); + + for (const ref of ifTestRefs) { + if (isState(ref)) { + context.report({ + node: ref.identifier as unknown as EsTreeNode, + message: + "Avoid using state and effects as an event handler. Instead, call the event handling code directly when the event occurs.", + }); + } + } + for (const ref of ifTestRefs) { + if (isProp(analysis, ref)) { + context.report({ + node: ref.identifier as unknown as EsTreeNode, + message: + "Avoid using props and effects as an event handler. Instead, move the handler to the parent component.", + }); + } + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-initialize-state.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-initialize-state.ts new file mode 100644 index 000000000..1a966d1bc --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-initialize-state.ts @@ -0,0 +1,70 @@ +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 { isNodeOfType } from "../../utils/is-node-of-type.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { getCallExpr, isSynchronous } from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + getEffectDepsRefs, + getEffectFn, + getEffectFnRefs, + getUseStateDecl, + isStateSetter, + isStateSetterCall, + isUseEffect, +} from "./utils/effect/react.js"; +import { stringifyExpressionSnippet } from "./utils/effect/stringify-expression-snippet.js"; + +// 1:1 port of upstream `src/rules/no-initialize-state.js`. +// Difference vs upstream: upstream uses `context.sourceCode.getText` +// for the diagnostic's "arguments" field; we use +// `stringifyExpressionSnippet` since oxlint plugins don't expose +// source text. Output text matches upstream byte-for-byte on the +// canonical literal / identifier / call shapes; falls back to +// `` for complex inputs. + +export const noInitializeState = defineRule({ + id: "no-initialize-state", + severity: "warn", + recommendation: + "Pass the initial value directly to useState() instead of setting it from a mount-only useEffect. For SSR hydration, prefer useSyncExternalStore().", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + const depsRefs = getEffectDepsRefs(analysis, node); + if (!effectFnRefs || !depsRefs) return; + const effectFn = getEffectFn(node); + if (!effectFn) return; + + const isEffectRunOnlyOnMount = depsRefs.filter((ref) => !isStateSetter(ref)).length === 0; + if (!isEffectRunOnlyOnMount) return; + + for (const ref of effectFnRefs) { + if (!isStateSetterCall(analysis, ref)) continue; + if (!isSynchronous(ref.identifier as unknown as EsTreeNode, effectFn)) continue; + const callExpr = getCallExpr(ref); + if (!callExpr || !isNodeOfType(callExpr, "CallExpression")) continue; + const useStateDecl = getUseStateDecl(analysis, ref); + if (!useStateDecl || !isNodeOfType(useStateDecl, "VariableDeclarator")) continue; + if (!isNodeOfType(useStateDecl.id, "ArrayPattern")) continue; + const elements = useStateDecl.id.elements ?? []; + const stateBinding = elements[0] ?? elements[1]; + const stateName = + stateBinding && isNodeOfType(stateBinding, "Identifier") ? stateBinding.name : ""; + const argumentText = + callExpr.arguments && callExpr.arguments.length > 0 + ? stringifyExpressionSnippet(callExpr.arguments[0] as EsTreeNode) + : "undefined"; + context.report({ + node: callExpr, + message: `Avoid initializing state in an effect. Instead, initialize "${stateName}"'s \`useState()\` with "${argumentText}". For SSR hydration, prefer \`useSyncExternalStore()\`.`, + }); + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-pass-data-to-parent.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-pass-data-to-parent.ts new file mode 100644 index 000000000..4ebbc614c --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-pass-data-to-parent.ts @@ -0,0 +1,113 @@ +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 type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { + getArgsUpstreamRefs, + getCallExpr, + getUpstreamRefs, + isSynchronous, +} from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + findContainingNode, + getEffectFn, + getEffectFnRefs, + hasCleanup, + isConstant, + isCustomHook, + isProp, + isPropCall, + isRefCall, + isRefCurrent, + isUseEffect, +} from "./utils/effect/react.js"; +import { isNodeOfType } from "../../utils/is-node-of-type.js"; + +// 1:1 port of upstream `src/rules/no-pass-data-to-parent.js`. + +// Local mirror of upstream's inline `isUseState`/`isUseRef` checks +// that work on the *identifier* of an upstream ref (not on a ref). +const isUseStateIdentifier = (identifier: EsTreeNode): boolean => { + if (!isNodeOfType(identifier, "Identifier")) return false; + if (identifier.name === "useState") return true; + const parent = (identifier as unknown as { parent?: EsTreeNode | null }).parent; + if ( + parent && + isNodeOfType(parent, "MemberExpression") && + isNodeOfType(parent.object, "Identifier") && + parent.object.name === "React" && + isNodeOfType(parent.property, "Identifier") && + parent.property.name === "useState" + ) { + return true; + } + return false; +}; + +const isUseRefIdentifier = (identifier: EsTreeNode): boolean => { + if (!isNodeOfType(identifier, "Identifier")) return false; + if (identifier.name === "useRef") return true; + const parent = (identifier as unknown as { parent?: EsTreeNode | null }).parent; + if ( + parent && + isNodeOfType(parent, "MemberExpression") && + isNodeOfType(parent.object, "Identifier") && + parent.object.name === "React" && + isNodeOfType(parent.property, "Identifier") && + parent.property.name === "useRef" + ) { + return true; + } + return false; +}; + +export const noPassDataToParent = defineRule({ + id: "no-pass-data-to-parent", + severity: "warn", + recommendation: + "Fetch the data in the parent and pass it to the child as a prop (or return it from the hook), instead of pushing it back up via a prop callback inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#passing-data-to-the-parent", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node) || hasCleanup(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + if (!effectFnRefs) return; + const effectFn = getEffectFn(node); + if (!effectFn) return; + + for (const ref of effectFnRefs) { + if (!isPropCall(analysis, ref)) continue; + if (isRefCall(analysis, ref)) continue; + if (!isSynchronous(ref.identifier as unknown as EsTreeNode, effectFn)) continue; + const callExpr = getCallExpr(ref); + if (!callExpr) continue; + + const argsUpstreamRefs = getArgsUpstreamRefs(analysis, ref).filter( + (argRef) => getUpstreamRefs(analysis, argRef).length === 1, + ); + + const isSomeArgsData = argsUpstreamRefs.some((argRef) => { + if (isUseStateIdentifier(argRef.identifier as unknown as EsTreeNode)) return false; + if (isProp(analysis, argRef)) return false; + if (isUseRefIdentifier(argRef.identifier as unknown as EsTreeNode)) return false; + if (isRefCurrent(argRef)) return false; + if (isConstant(argRef)) return false; + return true; + }); + if (!isSomeArgsData) continue; + + const containing = findContainingNode(analysis, node); + const isInCustomHook = containing != null && isCustomHook(containing); + context.report({ + node: callExpr, + message: isInCustomHook + ? "Avoid passing data to parents in an effect. Instead, return the data from the hook." + : "Avoid passing data to parents in an effect. Instead, fetch the data in the parent and pass it down to the child as a prop.", + }); + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-pass-live-state-to-parent.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-pass-live-state-to-parent.ts new file mode 100644 index 000000000..be0b954e7 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-pass-live-state-to-parent.ts @@ -0,0 +1,55 @@ +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 type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { getArgsUpstreamRefs, getCallExpr, isSynchronous } from "./utils/effect/ast.js"; +import { getProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + findContainingNode, + getEffectFn, + getEffectFnRefs, + isCustomHook, + isPropCall, + isState, + isUseEffect, +} from "./utils/effect/react.js"; + +// 1:1 port of upstream `src/rules/no-pass-live-state-to-parent.js`. + +export const noPassLiveStateToParent = defineRule({ + id: "no-pass-live-state-to-parent", + severity: "warn", + recommendation: + "Lift the state to the parent (or return it from the hook) instead of pushing it back up via a prop callback inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + if (!effectFnRefs) return; + const effectFn = getEffectFn(node); + if (!effectFn) return; + + for (const ref of effectFnRefs) { + if (!isPropCall(analysis, ref)) continue; + if (!isSynchronous(ref.identifier as unknown as EsTreeNode, effectFn)) continue; + const callExpr = getCallExpr(ref); + if (!callExpr) continue; + + const isStateInArgs = getArgsUpstreamRefs(analysis, ref).some((argRef) => isState(argRef)); + if (!isStateInArgs) continue; + + const containing = findContainingNode(analysis, node); + const isInCustomHook = containing != null && isCustomHook(containing); + context.report({ + node: callExpr, + message: isInCustomHook + ? "Avoid passing live state to parents in an effect. Instead, return the state from the hook." + : "Avoid passing live state to parents in an effect. Instead, lift the state to the parent and pass it down to the child as a prop.", + }); + } + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-reset-all-state-on-prop-change.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-reset-all-state-on-prop-change.ts new file mode 100644 index 000000000..5e3eff7f1 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-reset-all-state-on-prop-change.ts @@ -0,0 +1,112 @@ +import type { Reference } from "eslint-scope"; +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 { isNodeOfType } from "../../utils/is-node-of-type.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { getCallExpr, getDownstreamRefs, getUpstreamRefs } from "./utils/effect/ast.js"; +import { getProgramAnalysis, type ProgramAnalysis } from "./utils/effect/get-program-analysis.js"; +import { + findContainingNode, + getEffectDepsRefs, + getEffectFnRefs, + getUseStateDecl, + isCustomHook, + isProp, + isState, + isStateSetterCall, + isUseEffect, +} from "./utils/effect/react.js"; + +// 1:1 port of upstream `src/rules/no-reset-all-state-on-prop-change.js`. + +const isUndefinedNode = (node: EsTreeNode | null | undefined): boolean => { + if (node === null || node === undefined) return true; + return isNodeOfType(node, "Identifier") && node.name === "undefined"; +}; + +const getNodeText = (node: EsTreeNode | null | undefined): string => { + if (!node) return ""; + return JSON.stringify(node, (key, value) => { + if (key === "parent" || key === "loc" || key === "range" || key === "start" || key === "end") { + return undefined; + } + return value; + }); +}; + +const isSetStateToInitialValue = (analysis: ProgramAnalysis, setterRef: Reference): boolean => { + const callExpr = getCallExpr(setterRef); + if (!callExpr || !isNodeOfType(callExpr, "CallExpression")) return false; + const setStateToValue: EsTreeNode | undefined = callExpr.arguments?.[0] as EsTreeNode | undefined; + const useStateDecl = getUseStateDecl(analysis, setterRef); + if (!useStateDecl || !isNodeOfType(useStateDecl, "VariableDeclarator")) return false; + if (!isNodeOfType(useStateDecl.init, "CallExpression")) return false; + const stateInitialValue = useStateDecl.init.arguments?.[0] as EsTreeNode | undefined; + + if (isUndefinedNode(setStateToValue) && isUndefinedNode(stateInitialValue)) return true; + if (setStateToValue == null && stateInitialValue == null) return true; + if ((setStateToValue && !stateInitialValue) || (!setStateToValue && stateInitialValue)) { + return false; + } + return getNodeText(setStateToValue) === getNodeText(stateInitialValue); +}; + +const countUseStates = (analysis: ProgramAnalysis, componentNode: EsTreeNode | null): number => { + if (!componentNode) return 0; + let count = 0; + for (const ref of getDownstreamRefs(analysis, componentNode)) { + if (isState(ref)) count += 1; + } + return count; +}; + +const findPropUsedToResetAllState = ( + analysis: ProgramAnalysis, + effectFnRefs: Reference[], + depsRefs: Reference[], + useEffectNode: EsTreeNode, +): Reference | null => { + const stateSetterRefs = effectFnRefs.filter((ref) => isStateSetterCall(analysis, ref)); + if (stateSetterRefs.length === 0) return null; + + const allResetToInitial = stateSetterRefs.every((ref) => isSetStateToInitialValue(analysis, ref)); + if (!allResetToInitial) return null; + + const containing = findContainingNode(analysis, useEffectNode); + if (stateSetterRefs.length !== countUseStates(analysis, containing)) return null; + + for (const depRef of depsRefs) { + for (const upRef of getUpstreamRefs(analysis, depRef)) { + if (isProp(analysis, upRef)) return upRef; + } + } + return null; +}; + +export const noResetAllStateOnPropChange = defineRule({ + id: "no-reset-all-state-on-prop-change", + severity: "warn", + recommendation: + "Pass the prop as `key` so React resets the component when the prop changes, instead of manually resetting every state value in a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#resetting-all-state-when-a-prop-changes", + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isUseEffect(node)) return; + const analysis = getProgramAnalysis(node); + if (!analysis) return; + const effectFnRefs = getEffectFnRefs(analysis, node); + const depsRefs = getEffectDepsRefs(analysis, node); + if (!effectFnRefs || !depsRefs) return; + const containing = findContainingNode(analysis, node); + if (containing && isCustomHook(containing)) return; + + const propUsedToReset = findPropUsedToResetAllState(analysis, effectFnRefs, depsRefs, node); + if (!propUsedToReset) return; + context.report({ + node, + message: `Avoid resetting all state when a prop changes. Instead, if "${propUsedToReset.identifier.name}" is a key, pass it as \`key\` so React will reset the component's state.`, + }); + }, + }), +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/ast.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/ast.ts new file mode 100644 index 000000000..88692b887 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/ast.ts @@ -0,0 +1,186 @@ +import * as eslintVisitorKeys from "eslint-visitor-keys"; +import type { Reference } from "eslint-scope"; +import type { EsTreeNode } from "../../../../utils/es-tree-node.js"; +import { isAstNode } from "../../../../utils/is-ast-node.js"; +import { isNodeOfType } from "../../../../utils/is-node-of-type.js"; +import { getScopeForNode, type ProgramAnalysis } from "./get-program-analysis.js"; + +// 1:1 port of upstream `src/util/ast.js` from +// `eslint-plugin-react-you-might-not-need-an-effect`. Upstream uses +// `context.sourceCode.getScope(node)` and `context.sourceCode.visitorKeys`. +// We thread the cached `ProgramAnalysis` (eslint-scope ScopeManager) +// through every helper and use `eslint-visitor-keys.KEYS` as the +// static visitorKeys table. + +// Use the union of KEYS plus the JSX additions so we walk JSX children +// the same way ESLint would. +const VISITOR_KEYS: Readonly>> = { + ...eslintVisitorKeys.KEYS, +}; + +const ascend = ( + analysis: ProgramAnalysis, + ref: Reference, + visit: (ref: Reference) => boolean | undefined | void, + visited: Set = new Set(), +): void => { + if (visited.has(ref)) return; + const result = visit(ref); + visited.add(ref); + if (result === false) return; + + const defs = ref.resolved?.defs ?? []; + for (const def of defs) { + // Skip imports — terminate at the previous reference (actually + // using the imported thing). + if (def.type === "ImportBinding") continue; + // Skip parameters — their definition node is the function, so + // downstream would include the whole function body. + if (def.type === "Parameter") continue; + const defNode = def.node as unknown as Record; + const next = (defNode.init ?? defNode.body) as EsTreeNode | undefined; + if (!next) continue; + for (const innerRef of getDownstreamRefs(analysis, next)) { + ascend(analysis, innerRef, visit, visited); + } + } +}; + +const descend = ( + node: EsTreeNode, + visit: (node: EsTreeNode) => void, + visited: Set = new Set(), +): void => { + if (visited.has(node)) return; + visit(node); + visited.add(node); + + const keys = VISITOR_KEYS[node.type] ?? []; + const record = node as unknown as Record; + for (const key of keys) { + const child = record[key]; + if (!child) continue; + if (Array.isArray(child)) { + for (const item of child) { + if (item && isAstNode(item)) descend(item as EsTreeNode, visit, visited); + } + } else if (isAstNode(child)) { + descend(child as EsTreeNode, visit, visited); + } + } +}; + +export const getUpstreamRefs = (analysis: ProgramAnalysis, ref: Reference): Reference[] => { + const refs: Reference[] = []; + ascend(analysis, ref, (upRef) => { + refs.push(upRef); + }); + return refs; +}; + +export const findDownstreamNodes = (topNode: EsTreeNode, type: string): EsTreeNode[] => { + const nodes: EsTreeNode[] = []; + descend(topNode, (node) => { + if (node.type === type) nodes.push(node); + }); + return nodes; +}; + +export const getRef = (analysis: ProgramAnalysis, identifier: EsTreeNode): Reference | null => { + const scope = getScopeForNode(identifier, analysis.scopeManager); + if (!scope) return null; + for (const reference of scope.references) { + if (reference.identifier === identifier) return reference; + } + return null; +}; + +export const getDownstreamRefs = (analysis: ProgramAnalysis, node: EsTreeNode): Reference[] => { + const refs: Reference[] = []; + for (const identifier of findDownstreamNodes(node, "Identifier")) { + const ref = getRef(analysis, identifier); + if (ref) refs.push(ref); + } + return refs; +}; + +// Mirrors upstream's `getCallExpr(ref, current = ref.identifier.parent)`. +// Walks up MemberExpression chains so that for `obj.method()` the +// reference to `obj` resolves to the CallExpression. Returns null +// (instead of undefined) for symmetry with the rest of our helpers. +export const getCallExpr = ( + ref: Reference, + current: EsTreeNode | null | undefined = ( + ref.identifier as unknown as { parent?: EsTreeNode | null } + ).parent, +): EsTreeNode | null => { + if (!current) return null; + if (isNodeOfType(current, "CallExpression")) { + let node: EsTreeNode = ref.identifier as unknown as EsTreeNode; + let parent: EsTreeNode | null | undefined = (node as unknown as { parent?: EsTreeNode | null }) + .parent; + while (parent && isNodeOfType(parent, "MemberExpression")) { + node = parent; + parent = (node as unknown as { parent?: EsTreeNode | null }).parent; + } + if (current.callee === (node as unknown as typeof current.callee)) { + return current; + } + } + if (isNodeOfType(current, "MemberExpression")) { + return getCallExpr(ref, current.parent as EsTreeNode | undefined); + } + return null; +}; + +export const getArgsUpstreamRefs = (analysis: ProgramAnalysis, ref: Reference): Reference[] => { + const result: Reference[] = []; + for (const upRef of getUpstreamRefs(analysis, ref)) { + const callExpr = getCallExpr(upRef); + if (!callExpr || !isNodeOfType(callExpr, "CallExpression")) continue; + for (const argument of callExpr.arguments ?? []) { + for (const argRef of getDownstreamRefs(analysis, argument as EsTreeNode)) { + for (const innerRef of getUpstreamRefs(analysis, argRef)) { + result.push(innerRef); + } + } + } + } + return result; +}; + +// Mirrors upstream `isSynchronous`. Uses live AST parent pointers +// (which oxlint provides intact during visitor callbacks). +export const isSynchronous = (node: EsTreeNode | null | undefined, within: EsTreeNode): boolean => { + if (!node) return false; + if (node === within) return true; + const record = node as unknown as Record; + if (record.async === true) return false; + if ( + isNodeOfType(node, "AwaitExpression") || + (isNodeOfType(node, "UnaryExpression") && node.operator === "void") || + isNodeOfType(node, "FunctionDeclaration") || + isNodeOfType(node, "FunctionExpression") || + isNodeOfType(node, "ArrowFunctionExpression") + ) { + return false; + } + return isSynchronous(node.parent, within); +}; + +export const isEventualCallTo = ( + analysis: ProgramAnalysis, + ref: Reference, + predicate: (ref: Reference) => boolean, +): boolean => { + const callExprRefs: Reference[] = []; + ascend(analysis, ref, (upRef) => { + const callExpr = getCallExpr(upRef); + if (callExpr) { + callExprRefs.push(upRef); + } else { + return false; + } + }); + return callExprRefs.some(predicate); +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/get-program-analysis.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/get-program-analysis.ts new file mode 100644 index 000000000..47310959f --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/get-program-analysis.ts @@ -0,0 +1,122 @@ +import { analyze, type Scope, type ScopeManager } from "eslint-scope"; +import type { EsTreeNode } from "../../../../utils/es-tree-node.js"; +import type { EsTreeNodeOfType } from "../../../../utils/es-tree-node-of-type.js"; +import { isNodeOfType } from "../../../../utils/is-node-of-type.js"; + +export interface ProgramAnalysis { + programNode: EsTreeNodeOfType<"Program">; + scopeManager: ScopeManager; +} + +// HACK: WeakMap keyed on the live Program node so all 8 effect rules +// share a single eslint-scope analysis per file. The analysis is built +// lazily on first access from any rule. +const programToAnalysis: WeakMap = new WeakMap(); + +const findProgramRoot = (node: EsTreeNode): EsTreeNodeOfType<"Program"> | null => { + let cursor: EsTreeNode | null | undefined = node; + while (cursor) { + if (isNodeOfType(cursor, "Program")) return cursor; + cursor = cursor.parent; + } + return null; +}; + +// Strips `.parent` from every node in the subtree and remembers the +// originals so we can restore them after eslint-scope's `analyze()` +// returns. eslint-scope walks the tree internally; if any node still +// has a `parent` reference back up, it falls into infinite recursion +// (verified — `RangeError: Maximum call stack size exceeded`). +const stripAndRecordParents = ( + root: EsTreeNode, +): Array<{ node: Record; originalParent: unknown }> => { + const restorations: Array<{ node: Record; originalParent: unknown }> = []; + const seen = new WeakSet(); + const visit = (value: unknown): void => { + if (!value || typeof value !== "object") return; + if (seen.has(value as object)) return; + seen.add(value as object); + if (Array.isArray(value)) { + for (const item of value) visit(item); + return; + } + const record = value as Record; + if (!("type" in record)) return; + if ("parent" in record) { + restorations.push({ node: record, originalParent: record.parent }); + record.parent = null; + } + for (const key of Object.keys(record)) { + if (key === "parent") continue; + visit(record[key]); + } + }; + visit(root); + return restorations; +}; + +const restoreParents = ( + restorations: ReadonlyArray<{ node: Record; originalParent: unknown }>, +): void => { + for (const restoration of restorations) { + restoration.node.parent = restoration.originalParent; + } +}; + +// Returns the program-level eslint-scope analysis, caching per program +// so repeated calls within the same file (across multiple rules) reuse +// the work. Safe to call from any rule visitor callback — the in-place +// parent-strip + restore happens synchronously within this function. +// +// Returns `null` only if we can't find a Program root via the live +// parent chain (shouldn't happen in practice — defensive). +export const getProgramAnalysis = (anyNode: EsTreeNode): ProgramAnalysis | null => { + const programNode = findProgramRoot(anyNode); + if (!programNode) return null; + + const cached = programToAnalysis.get(programNode); + if (cached) return cached; + + const restorations = stripAndRecordParents(programNode); + let scopeManager: ScopeManager; + try { + scopeManager = analyze( + programNode as unknown as Parameters[0], + { + ecmaVersion: 2024, + sourceType: "module", + } as Parameters[1], + ); + } finally { + restoreParents(restorations); + } + + const analysis: ProgramAnalysis = { programNode, scopeManager }; + programToAnalysis.set(programNode, analysis); + return analysis; +}; + +// Replicates upstream's `context.sourceCode.getScope(node)`: returns the +// innermost scope that *contains* `node`. We find the deepest scope +// whose `block.range` strictly contains `node.range` (or whose `block` +// IS the node). +export const getScopeForNode = (node: EsTreeNode, manager: ScopeManager): Scope | null => { + if (!node.range) return null; + let bestScope: Scope | null = null; + let bestSize = Infinity; + for (const scope of manager.scopes) { + const block = scope.block as unknown as EsTreeNode; + if (!block?.range) continue; + if (node.range[0] < block.range[0] || node.range[1] > block.range[1]) continue; + const size = block.range[1] - block.range[0]; + // `<=` so that when two scopes have identical ranges (the + // global + module pair always share the Program range), the + // later-created (i.e. inner) scope wins — module-level + // declarations live in the module scope, not the global one. + if (size <= bestSize) { + bestSize = size; + bestScope = scope; + } + } + return bestScope; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/react.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/react.ts new file mode 100644 index 000000000..5666ffe19 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/react.ts @@ -0,0 +1,375 @@ +import type { Reference, Scope } from "eslint-scope"; +import type { EsTreeNode } from "../../../../utils/es-tree-node.js"; +import type { EsTreeNodeOfType } from "../../../../utils/es-tree-node-of-type.js"; +import { isNodeOfType } from "../../../../utils/is-node-of-type.js"; +import { getDownstreamRefs, getUpstreamRefs, isEventualCallTo } from "./ast.js"; +import type { ProgramAnalysis } from "./get-program-analysis.js"; + +const getOuterScopeContaining = (analysis: ProgramAnalysis, node: EsTreeNode): Scope | null => { + if (!node.range) return null; + // Find the smallest scope whose block strictly *contains* `node` + // (block.range fully envelops node.range). For a top-level + // VariableDeclarator in the module scope, this returns the + // module scope. + let best: Scope | null = null; + let bestSize = Infinity; + for (const scope of analysis.scopeManager.scopes) { + const block = scope.block as unknown as EsTreeNode; + if (!block?.range) continue; + if (node.range[0] < block.range[0] || node.range[1] > block.range[1]) continue; + const size = block.range[1] - block.range[0]; + // `<=` so that when two scopes have identical ranges (the + // global + module pair always share the Program range), the + // later-created (i.e. inner) scope wins — module variables live + // there, not in the global scope. + if (size <= bestSize) { + bestSize = size; + best = scope; + } + } + return best; +}; + +// 1:1 port of upstream `src/util/react.js` from +// `eslint-plugin-react-you-might-not-need-an-effect`. See `./ast.ts` +// for the matching analyzer-side port. + +const KNOWN_PURE_HOC_NAMES = new Set(["memo", "forwardRef"]); + +const startsWithUppercase = (name: string | undefined): boolean => + Boolean(name && name.length > 0 && name[0] >= "A" && name[0] <= "Z"); + +const isFunctionLike = ( + node: EsTreeNode | null | undefined, +): node is + | EsTreeNodeOfType<"ArrowFunctionExpression"> + | EsTreeNodeOfType<"FunctionExpression"> + | EsTreeNodeOfType<"FunctionDeclaration"> => + Boolean( + node && + (isNodeOfType(node, "ArrowFunctionExpression") || + isNodeOfType(node, "FunctionExpression") || + isNodeOfType(node, "FunctionDeclaration")), + ); + +export const isReactFunctionalComponent = (node: EsTreeNode | null | undefined): boolean => { + if (!node) return false; + if (isNodeOfType(node, "FunctionDeclaration")) { + return Boolean(node.id && startsWithUppercase(node.id.name)); + } + if (isNodeOfType(node, "VariableDeclarator")) { + if (!isNodeOfType(node.id, "Identifier")) return false; + if (!startsWithUppercase(node.id.name)) return false; + const init = node.init; + if (!init) return false; + return isNodeOfType(init, "ArrowFunctionExpression") || isNodeOfType(init, "CallExpression"); + } + return false; +}; + +export const isReactFunctionalHOC = ( + analysis: ProgramAnalysis, + node: EsTreeNode | null | undefined, +): boolean => { + if (!isReactFunctionalComponent(node)) return false; + if (!isNodeOfType(node, "VariableDeclarator")) return false; + const init = node.init; + if (!init) return false; + + // inline: `const MyComponent = withRouter(() => ...)` + const isWrappedInline = (): boolean => { + if (!isNodeOfType(init, "CallExpression")) return false; + if (!isNodeOfType(init.callee, "Identifier")) return false; + if (KNOWN_PURE_HOC_NAMES.has(init.callee.name)) return false; + const firstArg = init.arguments?.[0]; + if (!firstArg) return false; + return ( + isNodeOfType(firstArg, "ArrowFunctionExpression") || + isNodeOfType(firstArg, "FunctionExpression") + ); + }; + + // separately: `export default withRouter(MyComponent);` and + // `const Wrapped = inject('x')(observer(MyComponent))`. + // We find the Variable for `MyComponent` directly through the + // scope manager (instead of relying on `getRef(node.id)` resolving + // the LHS init reference, which depends on scope-analyzer + // particulars) and inspect each of its references. + const isWrappedSeparately = (): boolean => { + if (!isNodeOfType(node.id, "Identifier")) return false; + const bindingName = node.id.name; + const containingScope = getOuterScopeContaining(analysis, node as unknown as EsTreeNode); + if (!containingScope) return false; + const variable = containingScope.variables.find((v) => v.name === bindingName); + if (!variable) return false; + for (const reference of variable.references) { + const parent = (reference.identifier as unknown as { parent?: EsTreeNode | null }).parent; + if (!parent || !isNodeOfType(parent, "CallExpression")) continue; + const args = parent.arguments ?? []; + const refId = reference.identifier as unknown as (typeof args)[number]; + if (!args.includes(refId)) continue; + const callee = parent.callee; + const calleeName = isNodeOfType(callee, "Identifier") + ? callee.name + : isNodeOfType(callee, "CallExpression") && isNodeOfType(callee.callee, "Identifier") + ? callee.callee.name + : null; + if (calleeName != null && !KNOWN_PURE_HOC_NAMES.has(calleeName)) { + return true; + } + } + return false; + }; + + return isWrappedInline() || isWrappedSeparately(); +}; + +export const isCustomHook = (node: EsTreeNode | null | undefined): boolean => { + if (!node) return false; + if (isNodeOfType(node, "FunctionDeclaration")) { + const name = node.id?.name; + if (!name) return false; + return name.startsWith("use") && name.length > 3 && name[3] >= "A" && name[3] <= "Z"; + } + if (isNodeOfType(node, "VariableDeclarator")) { + if (!isNodeOfType(node.id, "Identifier")) return false; + const name = node.id.name; + const init = node.init; + if (!init) return false; + if ( + !isNodeOfType(init, "ArrowFunctionExpression") && + !isNodeOfType(init, "FunctionExpression") + ) { + return false; + } + return name.startsWith("use") && name.length > 3 && name[3] >= "A" && name[3] <= "Z"; + } + return false; +}; + +const isUseStateNode = (node: EsTreeNode | null | undefined): boolean => { + if (!node) return false; + if (isNodeOfType(node, "Identifier")) { + if (node.name === "useState") return true; + const parent = (node as unknown as { parent?: EsTreeNode | null }).parent; + if ( + parent && + isNodeOfType(parent, "MemberExpression") && + isNodeOfType(parent.object, "Identifier") && + parent.object.name === "React" && + isNodeOfType(parent.property, "Identifier") && + parent.property.name === "useState" + ) { + return true; + } + return false; + } + if (isNodeOfType(node, "MemberExpression")) { + return ( + isNodeOfType(node.object, "Identifier") && + node.object.name === "React" && + isNodeOfType(node.property, "Identifier") && + node.property.name === "useState" + ); + } + return false; +}; + +export const isUseEffect = (node: EsTreeNode | null | undefined): boolean => { + if (!node || !isNodeOfType(node, "CallExpression")) return false; + const callee = node.callee; + if (isNodeOfType(callee, "Identifier") && callee.name === "useEffect") return true; + if ( + isNodeOfType(callee, "MemberExpression") && + isNodeOfType(callee.object, "Identifier") && + callee.object.name === "React" && + isNodeOfType(callee.property, "Identifier") && + callee.property.name === "useEffect" + ) { + return true; + } + return false; +}; + +export const getEffectFn = (node: EsTreeNode): EsTreeNode | null => { + if (!isNodeOfType(node, "CallExpression")) return null; + const fn = node.arguments?.[0]; + if (!fn) return null; + if (!isNodeOfType(fn, "ArrowFunctionExpression") && !isNodeOfType(fn, "FunctionExpression")) { + return null; + } + return fn as EsTreeNode; +}; + +export const getEffectFnRefs = ( + analysis: ProgramAnalysis, + node: EsTreeNode, +): Reference[] | null => { + const fn = getEffectFn(node); + if (!fn) return null; + return getDownstreamRefs(analysis, fn); +}; + +export const getEffectDepsRefs = ( + analysis: ProgramAnalysis, + node: EsTreeNode, +): Reference[] | null => { + if (!isNodeOfType(node, "CallExpression")) return null; + const deps = node.arguments?.[1]; + if (!deps || !isNodeOfType(deps, "ArrayExpression")) return null; + return getDownstreamRefs(analysis, deps as EsTreeNode); +}; + +export const isState = (ref: Reference): boolean => + Boolean( + ref.resolved?.defs.some((def) => { + const node = def.node as unknown as EsTreeNode; + if (!isNodeOfType(node, "VariableDeclarator")) return false; + if (!isNodeOfType(node.init, "CallExpression")) return false; + if (!isUseStateNode(node.init.callee as EsTreeNode)) return false; + if (!isNodeOfType(node.id, "ArrayPattern")) return false; + const elements = node.id.elements ?? []; + if (elements.length !== 1 && elements.length !== 2) return false; + const first = elements[0]; + return Boolean( + first && isNodeOfType(first, "Identifier") && first.name === ref.identifier.name, + ); + }), + ); + +export const isStateSetter = (ref: Reference): boolean => + Boolean( + ref.resolved?.defs.some((def) => { + const node = def.node as unknown as EsTreeNode; + if (!isNodeOfType(node, "VariableDeclarator")) return false; + if (!isNodeOfType(node.init, "CallExpression")) return false; + if (!isUseStateNode(node.init.callee as EsTreeNode)) return false; + if (!isNodeOfType(node.id, "ArrayPattern")) return false; + const elements = node.id.elements ?? []; + if (elements.length !== 2) return false; + const second = elements[1]; + return Boolean( + second && isNodeOfType(second, "Identifier") && second.name === ref.identifier.name, + ); + }), + ); + +export const isProp = (analysis: ProgramAnalysis, ref: Reference): boolean => + Boolean( + ref.resolved?.defs.some((def) => { + if (def.type !== "Parameter") return false; + const defNode = def.node as unknown as EsTreeNode; + let declaringNode: EsTreeNode | null | undefined = defNode; + if (isNodeOfType(defNode, "ArrowFunctionExpression")) { + const parent = (defNode as unknown as { parent?: EsTreeNode | null }).parent; + if (parent && isNodeOfType(parent, "CallExpression")) { + declaringNode = (parent as unknown as { parent?: EsTreeNode | null }).parent; + } else { + declaringNode = parent; + } + } + if (!declaringNode) return false; + return ( + (isReactFunctionalComponent(declaringNode) && + !isReactFunctionalHOC(analysis, declaringNode)) || + isCustomHook(declaringNode) + ); + }), + ); + +export const isConstant = (ref: Reference): boolean => + Boolean( + (ref.resolved?.defs ?? []).some((def) => { + const node = def.node as unknown as EsTreeNode; + if (!isNodeOfType(node, "VariableDeclarator")) return false; + const init = node.init; + if (!init) return false; + return ( + isNodeOfType(init, "Literal") || + isNodeOfType(init, "TemplateLiteral") || + isNodeOfType(init, "ArrayExpression") || + isNodeOfType(init, "ObjectExpression") + ); + }), + ); + +export const isRef = (ref: Reference): boolean => + Boolean( + ref.resolved?.defs.some((def) => { + const node = def.node as unknown as EsTreeNode; + if (!isNodeOfType(node, "VariableDeclarator")) return false; + if (!isNodeOfType(node.init, "CallExpression")) return false; + const callee = node.init.callee; + if (isNodeOfType(callee, "Identifier") && callee.name === "useRef") return true; + if ( + isNodeOfType(callee, "MemberExpression") && + isNodeOfType(callee.object, "Identifier") && + callee.object.name === "React" && + isNodeOfType(callee.property, "Identifier") && + callee.property.name === "useRef" + ) { + return true; + } + return false; + }), + ); + +export const isRefCurrent = (ref: Reference): boolean => { + const parent = (ref.identifier as unknown as { parent?: EsTreeNode | null }).parent; + if (!parent || !isNodeOfType(parent, "MemberExpression")) return false; + if (!isNodeOfType(parent.property, "Identifier")) return false; + return parent.property.name === "current"; +}; + +export const isStateSetterCall = (analysis: ProgramAnalysis, ref: Reference): boolean => + isEventualCallTo(analysis, ref, isStateSetter); + +export const isPropCall = (analysis: ProgramAnalysis, ref: Reference): boolean => + isEventualCallTo(analysis, ref, (innerRef) => isProp(analysis, innerRef)); + +export const isRefCall = (analysis: ProgramAnalysis, ref: Reference): boolean => + isEventualCallTo(analysis, ref, (innerRef) => isRefCurrent(innerRef) || isRef(innerRef)); + +export const getUseStateDecl = (analysis: ProgramAnalysis, ref: Reference): EsTreeNode | null => { + const useStateRef = getUpstreamRefs(analysis, ref).find((upRef) => + isUseStateNode(upRef.identifier as unknown as EsTreeNode), + ); + let node: EsTreeNode | null | undefined = useStateRef?.identifier as unknown as EsTreeNode; + while (node && !isNodeOfType(node, "VariableDeclarator")) { + node = (node as unknown as { parent?: EsTreeNode | null }).parent; + } + return node ?? null; +}; + +export const hasCleanup = (node: EsTreeNode): boolean => { + const fn = node && isNodeOfType(node, "CallExpression") ? node.arguments?.[0] : null; + if (!fn) return false; + if (!isNodeOfType(fn, "ArrowFunctionExpression") && !isNodeOfType(fn, "FunctionExpression")) { + return false; + } + if (!isNodeOfType(fn.body, "BlockStatement")) return false; + return (fn.body.body ?? []).some( + (stmt) => isNodeOfType(stmt, "ReturnStatement") && stmt.argument != null, + ); +}; + +export const findContainingNode = ( + analysis: ProgramAnalysis, + node: EsTreeNode | null | undefined, +): EsTreeNode | null => { + if (!node) return null; + if ( + isReactFunctionalComponent(node) || + isReactFunctionalHOC(analysis, node) || + isCustomHook(node) + ) { + return node; + } + const parent = (node as unknown as { parent?: EsTreeNode | null }).parent; + return findContainingNode(analysis, parent); +}; + +// Re-export `isFunctionLike` so consumers (rules) and tests can use +// it without re-declaring; also keeps the imported helper from being +// reported as unused. +export { isFunctionLike }; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/stringify-expression-snippet.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/stringify-expression-snippet.ts new file mode 100644 index 000000000..afed9277e --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/utils/effect/stringify-expression-snippet.ts @@ -0,0 +1,93 @@ +import type { EsTreeNode } from "../../../../utils/es-tree-node.js"; +import { isNodeOfType } from "../../../../utils/is-node-of-type.js"; + +// Small AST snippet → string formatter used by `no-initialize-state` so +// the diagnostic can suggest `useState("Dr. " + name)` rather than just +// referencing the position. Conservative on purpose — anything we don't +// recognize falls back to `` so the message stays bounded. +// +// Not a full pretty-printer; we deliberately do not chase MemberExpression +// computed keys, AssignmentExpressions, ConditionalExpressions, or +// arbitrary nesting beyond a few common shapes. +const MAX_CALL_ARGS_DISPLAY = 3; +const FALLBACK = ""; + +const stringifyLiteral = (literal: { value?: unknown; raw?: string }): string => { + if (typeof literal.raw === "string") return literal.raw; + if (literal.value === null) return "null"; + if (typeof literal.value === "string") return JSON.stringify(literal.value); + if (typeof literal.value === "number") return String(literal.value); + if (typeof literal.value === "boolean") return String(literal.value); + return FALLBACK; +}; + +const stringifyTemplateLiteral = (node: EsTreeNode): string => { + if (!isNodeOfType(node, "TemplateLiteral")) return FALLBACK; + const quasis = node.quasis ?? []; + const expressions = node.expressions ?? []; + let out = "`"; + for (let i = 0; i < quasis.length; i += 1) { + out += quasis[i].value?.cooked ?? quasis[i].value?.raw ?? ""; + if (i < expressions.length) { + out += "${" + stringifyExpressionSnippet(expressions[i]) + "}"; + } + } + out += "`"; + return out; +}; + +export const stringifyExpressionSnippet = (node: EsTreeNode | null | undefined): string => { + if (!node) return "undefined"; + if (isNodeOfType(node, "Literal")) return stringifyLiteral(node); + if (isNodeOfType(node, "Identifier")) return node.name; + if (isNodeOfType(node, "TemplateLiteral")) return stringifyTemplateLiteral(node); + if (isNodeOfType(node, "MemberExpression")) { + if ( + isNodeOfType(node.object, "Identifier") && + isNodeOfType(node.property, "Identifier") && + !node.computed + ) { + return `${node.object.name}.${node.property.name}`; + } + return FALLBACK; + } + if (isNodeOfType(node, "CallExpression")) { + const calleeText = isNodeOfType(node.callee, "Identifier") + ? node.callee.name + : isNodeOfType(node.callee, "MemberExpression") && + isNodeOfType(node.callee.object, "Identifier") && + isNodeOfType(node.callee.property, "Identifier") && + !node.callee.computed + ? `${node.callee.object.name}.${node.callee.property.name}` + : FALLBACK; + const argText = (node.arguments ?? []) + .slice(0, MAX_CALL_ARGS_DISPLAY) + .map((argument) => stringifyExpressionSnippet(argument as EsTreeNode)) + .join(", "); + const suffix = (node.arguments?.length ?? 0) > MAX_CALL_ARGS_DISPLAY ? ", …" : ""; + return `${calleeText}(${argText}${suffix})`; + } + if (isNodeOfType(node, "ArrayExpression")) { + const items = (node.elements ?? []) + .slice(0, MAX_CALL_ARGS_DISPLAY) + .map((element) => (element ? stringifyExpressionSnippet(element) : "")) + .join(", "); + const suffix = (node.elements?.length ?? 0) > MAX_CALL_ARGS_DISPLAY ? ", …" : ""; + return `[${items}${suffix}]`; + } + if (isNodeOfType(node, "ObjectExpression")) { + if ((node.properties?.length ?? 0) === 0) return "{}"; + return "{ … }"; + } + if (isNodeOfType(node, "ArrowFunctionExpression")) return "() => …"; + if (isNodeOfType(node, "FunctionExpression")) return "function () { … }"; + if (isNodeOfType(node, "BinaryExpression") || isNodeOfType(node, "LogicalExpression")) { + const left = stringifyExpressionSnippet(node.left); + const right = stringifyExpressionSnippet(node.right); + return `${left} ${node.operator} ${right}`; + } + if (isNodeOfType(node, "UnaryExpression")) { + return `${node.operator}${stringifyExpressionSnippet(node.argument)}`; + } + return FALLBACK; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/rules.ts b/packages/oxlint-plugin-react-doctor/src/rules.ts index 41f54c019..a350b208c 100644 --- a/packages/oxlint-plugin-react-doctor/src/rules.ts +++ b/packages/oxlint-plugin-react-doctor/src/rules.ts @@ -51,26 +51,12 @@ export const EXTERNAL_RULES = [ { key: "react-hooks-js/void-use-memo", source: "react-compiler", severity: "error" }, { key: "react-hooks-js/incompatible-library", source: "react-compiler", severity: "error" }, { key: "react-hooks-js/todo", source: "react-compiler", severity: "error" }, - { key: "effect/no-derived-state", source: "you-might-not-need-effect", severity: "warn" }, - { key: "effect/no-chain-state-updates", source: "you-might-not-need-effect", severity: "warn" }, - { key: "effect/no-event-handler", source: "you-might-not-need-effect", severity: "warn" }, - { - key: "effect/no-adjust-state-on-prop-change", - source: "you-might-not-need-effect", - severity: "warn", - }, - { - key: "effect/no-reset-all-state-on-prop-change", - source: "you-might-not-need-effect", - severity: "warn", - }, - { - key: "effect/no-pass-live-state-to-parent", - source: "you-might-not-need-effect", - severity: "warn", - }, - { key: "effect/no-pass-data-to-parent", source: "you-might-not-need-effect", severity: "warn" }, - { key: "effect/no-initialize-state", source: "you-might-not-need-effect", severity: "warn" }, + // Note: the 8 `effect/*` rules from `eslint-plugin-react-you-might-not-need-an-effect` + // were previously listed here as external. They are now natively + // ported into this package under `react-doctor/*` (see + // `state-and-effects/no-derived-state.ts` and friends, plus + // `state-and-effects/effect/SOURCE.md`) and activate through the + // normal `REACT_DOCTOR_RULES` loop. { key: "react/rules-of-hooks", source: "builtin-react", severity: "error" }, { key: "react/no-direct-mutation-state", source: "builtin-react", severity: "error" }, { key: "react/jsx-no-duplicate-props", source: "builtin-react", severity: "error" }, @@ -112,8 +98,9 @@ export const ALL_REACT_DOCTOR_RULE_KEYS: ReadonlySet = new Set( ); export const FRAMEWORK_SPECIFIC_RULE_KEYS = collectFrameworkSpecificRuleKeys(); export const REACT_COMPILER_RULES = toRuleMap(collectExternalRulesBySource("react-compiler")); -export const YOU_MIGHT_NOT_NEED_EFFECT_RULES = toRuleMap( - collectExternalRulesBySource("you-might-not-need-effect"), -); +// Deprecated: kept as an empty map for back-compat. The 8 rules +// formerly listed here are now natively ported as `react-doctor/*`. +// Will be removed in a future major. +export const YOU_MIGHT_NOT_NEED_EFFECT_RULES: Record = {}; export const BUILTIN_REACT_RULES = toRuleMap(collectExternalRulesBySource("builtin-react")); export const BUILTIN_A11Y_RULES = toRuleMap(collectExternalRulesBySource("builtin-a11y")); diff --git a/packages/react-doctor/README.md b/packages/react-doctor/README.md index afd83ed2a..29e63d9e5 100644 --- a/packages/react-doctor/README.md +++ b/packages/react-doctor/README.md @@ -191,12 +191,13 @@ Each surface accepts `includeTags`, `excludeTags`, `includeCategories`, `exclude #### Optional companion plugins -When the following ESLint plugins are installed in the scanned project (or hoisted in your monorepo), React Doctor folds their rules into the same scan. Both are listed as **optional peer dependencies** — install only what you want. +When the following ESLint plugins are installed in the scanned project (or hoisted in your monorepo), React Doctor folds their rules into the same scan. Listed as **optional peer dependencies** — install only what you want. -| Plugin | Adds | Namespace | -| ----------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -| [`eslint-plugin-react-hooks`](https://www.npmjs.com/package/eslint-plugin-react-hooks) (v6 or v7) | The React Compiler frontend's correctness rules — fired when a React Compiler is detected in the project. | `react-hooks-js/*` | -| [`eslint-plugin-react-you-might-not-need-an-effect`](https://github.com/nickjvandyke/eslint-plugin-react-you-might-not-need-an-effect) (v0.10+) | Complementary effects-as-anti-pattern rules (`no-derived-state`, `no-chain-state-updates`, `no-event-handler`, `no-pass-data-to-parent`, …) that run alongside React Doctor's native State & Effects rules. | `effect/*` | +| Plugin | Adds | Namespace | +| ------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | ------------------ | +| [`eslint-plugin-react-hooks`](https://www.npmjs.com/package/eslint-plugin-react-hooks) (v6 or v7) | The React Compiler frontend's correctness rules — fired when a React Compiler is detected in the project. | `react-hooks-js/*` | + +The 8 rules from [`eslint-plugin-react-you-might-not-need-an-effect`](https://github.com/nickjvandyke/eslint-plugin-react-you-might-not-need-an-effect) (NickvanDyke, MIT) are now ported natively into React Doctor — they fire as `react-doctor/no-derived-state`, `react-doctor/no-chain-state-updates`, `react-doctor/no-event-handler`, `react-doctor/no-adjust-state-on-prop-change`, `react-doctor/no-reset-all-state-on-prop-change`, `react-doctor/no-pass-live-state-to-parent`, `react-doctor/no-pass-data-to-parent`, and `react-doctor/no-initialize-state`. No peer dependency required. ### Inline suppressions diff --git a/packages/react-doctor/package.json b/packages/react-doctor/package.json index 98b2910ca..d70976fd8 100644 --- a/packages/react-doctor/package.json +++ b/packages/react-doctor/package.json @@ -69,19 +69,14 @@ "@react-doctor/project-info": "workspace:*", "@react-doctor/types": "workspace:*", "@types/prompts": "^2.4.9", - "eslint-plugin-react-hooks": "^7.1.1", - "eslint-plugin-react-you-might-not-need-an-effect": "^0.10.1" + "eslint-plugin-react-hooks": "^7.1.1" }, "peerDependencies": { - "eslint-plugin-react-hooks": "^6 || ^7", - "eslint-plugin-react-you-might-not-need-an-effect": "^0.10" + "eslint-plugin-react-hooks": "^6 || ^7" }, "peerDependenciesMeta": { "eslint-plugin-react-hooks": { "optional": true - }, - "eslint-plugin-react-you-might-not-need-an-effect": { - "optional": true } }, "engines": { diff --git a/packages/react-doctor/tests/oxlint-batching.test.ts b/packages/react-doctor/tests/oxlint-batching.test.ts index 69cefb46f..ac8a42974 100644 --- a/packages/react-doctor/tests/oxlint-batching.test.ts +++ b/packages/react-doctor/tests/oxlint-batching.test.ts @@ -3,8 +3,9 @@ import { OXLINT_MAX_FILES_PER_BATCH, batchIncludePaths } from "@react-doctor/cor describe("OXLINT_MAX_FILES_PER_BATCH (perf cliff guard)", () => { // HACK: empirically verified that the upstream `effect` plugin - // (`eslint-plugin-react-you-might-not-need-an-effect`) hits the - // 5-min oxlint spawn timeout at batch=500 on supabase/studio's + // (`eslint-plugin-react-you-might-not-need-an-effect`, the source of + // the now-natively-ported `react-doctor/no-derived-state` family) + // hits the 5-min oxlint spawn timeout at batch=500 on supabase/studio's // ~3500 source files (returns 0 diagnostics, marks lint as skipped), // but completes in ~30s with batch=100. If a future bump pushes this // back above ~250, large-monorepo scans regress to silent timeout. diff --git a/packages/react-doctor/tests/regressions/scan-resilience.test.ts b/packages/react-doctor/tests/regressions/scan-resilience.test.ts index da037c231..c53927585 100644 --- a/packages/react-doctor/tests/regressions/scan-resilience.test.ts +++ b/packages/react-doctor/tests/regressions/scan-resilience.test.ts @@ -351,57 +351,36 @@ describe("issue #141: oxlint config must not reference unloaded plugins", () => } }); - it("loads eslint-plugin-react-you-might-not-need-an-effect when installed (#187)", () => { + it("ships the 8 ported `you-might-not-need-an-effect` rules as react-doctor rules", () => { + // After the native port (#187 follow-up), the previously-external + // `effect/*` rule surface lives inside `oxlint-plugin-react-doctor` + // as plain `react-doctor/*` global rules. No JS plugin entry, no + // separate `effect/` namespace, no optional peer dependency. const config = createOxlintConfig({ pluginPath: "/tmp/react-doctor-plugin.js", project: buildTestProject({ rootDirectory: "/tmp/test" }), }); - const effectRuleKeys = Object.keys(config.rules).filter((ruleKey) => - ruleKey.startsWith("effect/"), - ); - const hasEffectPluginEntry = config.jsPlugins.some( - (jsPlugin) => typeof jsPlugin === "object" && jsPlugin.name === "effect", - ); - - expect(hasEffectPluginEntry).toBe(true); - expect(effectRuleKeys.length).toBeGreaterThan(0); - expect(effectRuleKeys.every((ruleKey) => config.rules[ruleKey] === "warn")).toBe(true); - }); - - it("emits no effect/* rules when customRulesOnly skips third-party plugins (#187)", () => { - const config = createOxlintConfig({ - pluginPath: "/tmp/react-doctor-plugin.js", - project: buildTestProject({ rootDirectory: "/tmp/test" }), - customRulesOnly: true, - }); - - const effectRuleKeys = Object.keys(config.rules).filter((ruleKey) => - ruleKey.startsWith("effect/"), - ); - const hasEffectPluginEntry = config.jsPlugins.some( - (jsPlugin) => typeof jsPlugin === "object" && jsPlugin.name === "effect", - ); - - expect(effectRuleKeys).toHaveLength(0); - expect(hasEffectPluginEntry).toBe(false); - }); - - it("only enables effect/* rules that the resolved plugin actually exports (#187)", async () => { - const config = createOxlintConfig({ - pluginPath: "/tmp/react-doctor-plugin.js", - project: buildTestProject({ rootDirectory: "/tmp/test" }), - }); - const pluginModule = await import("eslint-plugin-react-you-might-not-need-an-effect"); - const availableRuleNames = new Set( - Object.keys((pluginModule.default ?? pluginModule).rules ?? {}), - ); - const enabledRuleNames = Object.keys(config.rules) - .filter((ruleKey) => ruleKey.startsWith("effect/")) - .map((ruleKey) => ruleKey.replace(/^effect\//, "")); - expect(enabledRuleNames.length).toBeGreaterThan(0); - for (const ruleName of enabledRuleNames) { - expect(availableRuleNames.has(ruleName)).toBe(true); + const portedRuleIds = [ + "no-derived-state", + "no-chain-state-updates", + "no-event-handler", + "no-adjust-state-on-prop-change", + "no-reset-all-state-on-prop-change", + "no-pass-live-state-to-parent", + "no-pass-data-to-parent", + "no-initialize-state", + ]; + for (const ruleId of portedRuleIds) { + const fullKey = `react-doctor/${ruleId}`; + expect(config.rules[fullKey]).toBe("warn"); } + + expect(Object.keys(config.rules).some((ruleKey) => ruleKey.startsWith("effect/"))).toBe(false); + expect( + config.jsPlugins.some( + (jsPlugin) => typeof jsPlugin === "object" && jsPlugin.name === "effect", + ), + ).toBe(false); }); }); diff --git a/packages/react-doctor/tests/regressions/state-rules/_effect-parity-runner.ts b/packages/react-doctor/tests/regressions/state-rules/_effect-parity-runner.ts new file mode 100644 index 000000000..4c96128da --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/_effect-parity-runner.ts @@ -0,0 +1,161 @@ +import fs from "node:fs"; +import path from "node:path"; +import url from "node:url"; +import { describe, expect, it } from "vite-plus/test"; +import { collectRuleHits, createScopedTempRoot, setupReactProject } from "./_helpers.js"; + +interface UpstreamCase { + idx: number; + name: string; + code: string; + todo: boolean; + errors?: number; +} + +interface UpstreamFixture { + valid: UpstreamCase[]; + invalid: UpstreamCase[]; +} + +const slugify = (input: string): string => + input + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-|-$/g, "") + .slice(0, 40) || "case"; + +const fixturesRoot = path.join(path.dirname(url.fileURLToPath(import.meta.url)), "effect-fixtures"); + +const loadFixture = (ruleId: string): UpstreamFixture => + JSON.parse(fs.readFileSync(path.join(fixturesRoot, `${ruleId}.json`), "utf8")); + +// Wraps each upstream `code` snippet in a `.tsx` file the synthetic +// React project can lint. Upstream code assumes `useState`, `useEffect` +// etc. as globals — we don't add any prelude / shims because doing so +// would create shadowing collisions (a `declare const Foo` next to the +// upstream `function Foo() {}` confuses eslint-scope's resolution). +// oxlint doesn't type-check, so undeclared references are tolerated. +// The rule recognizes `useState` / `useEffect` by Identifier name, +// not by resolved import, so this works. +const upstreamShimPrelude = ""; + +export interface RunUpstreamParityOptions { + /** + * Override the rule id we filter diagnostics by — defaults to the + * fixture name. Used by the `syntax` fixture, which captures cases + * upstream runs through the `no-derived-state` rule but lives under + * a different fixture filename. + */ + ruleId?: string; + /** + * When true, assert that NONE of the 8 ported `react-doctor/*` rules + * fire on each `valid:` case (instead of just filtering by `ruleId`). + * Mirrors upstream `real-world.test.js`, which runs the full + * `recommended` config and asserts no diagnostics. + */ + assertNoneOfPortedRules?: boolean; +} + +const PORTED_RULE_IDS: ReadonlyArray = [ + "no-derived-state", + "no-chain-state-updates", + "no-event-handler", + "no-adjust-state-on-prop-change", + "no-reset-all-state-on-prop-change", + "no-pass-live-state-to-parent", + "no-pass-data-to-parent", + "no-initialize-state", +]; + +const collectAnyPortedRuleHits = async ( + projectDir: string, +): Promise> => { + const aggregated: Array<{ rule: string; message: string }> = []; + for (const ruleId of PORTED_RULE_IDS) { + const hits = await collectRuleHits(projectDir, ruleId); + for (const hit of hits) { + aggregated.push({ rule: ruleId, message: hit.message }); + } + } + return aggregated; +}; + +export const runUpstreamParity = ( + fixtureName: string, + options: RunUpstreamParityOptions = {}, +): void => { + const ruleIdToFilter = options.ruleId ?? fixtureName; + const tempRoot = createScopedTempRoot(`effect-${fixtureName}-parity`); + const fixture = loadFixture(fixtureName); + + const wrapAsTsx = (code: string): string => { + return `${upstreamShimPrelude}\n// === upstream snippet ===\n${code}\n`; + }; + + describe(`${fixtureName} parity (port of eslint-plugin-react-you-might-not-need-an-effect)`, () => { + for (const validCase of fixture.valid) { + const itFn = validCase.todo ? it.skip : it; + itFn(`valid #${validCase.idx} "${validCase.name}"`, async () => { + const projectDir = setupReactProject( + tempRoot, + `v-${validCase.idx}-${slugify(validCase.name)}`, + { + files: { "src/Component.tsx": wrapAsTsx(validCase.code) }, + }, + ); + if (options.assertNoneOfPortedRules) { + const hits = await collectAnyPortedRuleHits(projectDir); + if (hits.length !== 0) { + const fs = await import("node:fs"); + fs.appendFileSync( + "/tmp/parity-failures.log", + `[${fixtureName}/any-ported] valid #${validCase.idx} "${validCase.name}" expected=0 got=${hits.length}\n code:\n${validCase.code + .split("\n") + .map((l) => ` ${l}`) + .join("\n")}\n hits:\n${JSON.stringify(hits, null, 2)}\n---\n`, + ); + } + expect(hits).toHaveLength(0); + return; + } + const hits = await collectRuleHits(projectDir, ruleIdToFilter); + if (hits.length !== 0) { + const fs = await import("node:fs"); + fs.appendFileSync( + "/tmp/parity-failures.log", + `[${ruleIdToFilter}] valid #${validCase.idx} "${validCase.name}" expected=0 got=${hits.length}\n code:\n${validCase.code + .split("\n") + .map((l) => ` ${l}`) + .join("\n")}\n hits:\n${JSON.stringify(hits, null, 2)}\n---\n`, + ); + } + expect(hits).toHaveLength(0); + }); + } + + for (const invalidCase of fixture.invalid) { + const itFn = invalidCase.todo ? it.skip : it; + itFn(`invalid #${invalidCase.idx} "${invalidCase.name}"`, async () => { + const projectDir = setupReactProject( + tempRoot, + `i-${invalidCase.idx}-${slugify(invalidCase.name)}`, + { + files: { "src/Component.tsx": wrapAsTsx(invalidCase.code) }, + }, + ); + const hits = await collectRuleHits(projectDir, ruleIdToFilter); + if (hits.length !== (invalidCase.errors ?? 1)) { + const fs = await import("node:fs"); + fs.appendFileSync( + "/tmp/parity-failures.log", + `[${ruleIdToFilter}] invalid #${invalidCase.idx} "${invalidCase.name}" expected=${invalidCase.errors ?? 1} got=${hits.length}\n code:\n${invalidCase.code + .split("\n") + .map((l) => ` ${l}`) + .join("\n")}\n hits:\n${JSON.stringify(hits, null, 2)}\n---\n`, + ); + } + expect(hits.length).toBe(invalidCase.errors ?? 1); + }); + } + }); +}; diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-adjust-state-on-prop-change.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-adjust-state-on-prop-change.json new file mode 100644 index 000000000..6e2030eae --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-adjust-state-on-prop-change.json @@ -0,0 +1,52 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Adjusting state directly during render", + "code": "\n function List({ items }) {\n const [isReverse, setIsReverse] = useState(false);\n const [selection, setSelection] = useState(null);\n\n const [prevItems, setPrevItems] = useState(items);\n if (items !== prevItems) {\n setPrevItems(items);\n setSelection(null);\n }\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Set state to literal when internal state changes", + "code": "\n function Counter() {\n const [count, setCount] = useState(0);\n const [otherState, setOtherState] = useState();\n\n useEffect(() => {\n setOtherState('Hello World');\n }, [count]);\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Set state to a value derived from props", + "code": "\n function Counter({ count }) {\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => {\n setDoubleCount(count * 2);\n }, [count]);\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Set state to literal when prop changes", + "code": "\n function List({ items }) {\n const [selection, setSelection] = useState();\n\n useEffect(() => {\n setSelection(null);\n }, [items]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Set state to internal state when prop changes", + "code": "\n function List({ items }) {\n const [selection, setSelection] = useState();\n const [internalData, setInternalData] = useState();\n\n useEffect(() => {\n setSelection(internalData);\n }, [items, internalData]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "Set state to external state when prop changes", + "code": "\n function List({ items }) {\n const [selection, setSelection] = useState();\n const { data: externalData } = useDataSource();\n\n useEffect(() => {\n setSelection(externalData);\n }, [items]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Conditionally set state to literal when prop changes", + "code": "\n function Form({ result }) {\n const [error, setError] = useState();\n\n useEffect(() => {\n if (result.data) {\n setError(null);\n }\n }, [result]);\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-chain-state-updates.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-chain-state-updates.json new file mode 100644 index 000000000..84b428420 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-chain-state-updates.json @@ -0,0 +1,78 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Set state to literal when props change", + "code": "\n function List({ items }) {\n const [selection, setSelection] = useState();\n\n useEffect(() => {\n setSelection(null);\n }, [items]);\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Set state to derived internal state when internal state changes", + "code": "\n function Counter() {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => {\n setDoubleCount(count * 2);\n }, [count]);\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Set state to literal when external state changes", + "code": "\n function Feed() {\n const { data: posts } = useQuery('/posts');\n const [scrollPosition, setScrollPosition] = useState(0);\n\n useEffect(() => {\n setScrollPosition(0);\n }, [posts]);\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "Synchronize internal state with literal", + "code": "\n function Component() {\n const [isMounted, setIsMounted] = useState(false);\n\n useEffect(() => {\n setIsMounted(true);\n return () => setIsMounted(false);\n }, [setIsMounted]);\n }\n ", + "todo": false + }, + { + "idx": 4, + "name": "Set state to internal plus external state", + "code": "\n function Game() {\n const [round, setRound] = useState(1);\n const [isGameOver, setIsGameOver] = useState(false);\n const { data: players } = useQuery('/players');\n\n useEffect(() => {\n setIsGameOver(round > 10 || players.length === 0);\n }, [round, players]);\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Set state to literal when internal state changes", + "code": "\n function Game() {\n const [round, setRound] = useState(1);\n const [isGameOver, setIsGameOver] = useState(false);\n\n useEffect(() => {\n if (round > 10) {\n setIsGameOver(true);\n }\n }, [round]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Set state to derived literal when internal state changes", + "code": "\n function Game() {\n const [round, setRound] = useState(1);\n const [isGameOver, setIsGameOver] = useState(false);\n\n useEffect(() => {\n if (round > 10) {\n const finalRound = true;\n setIsGameOver(finalRound);\n }\n }, [round]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "JSON.stringifying internal state in deps", + "code": "\n function Feed() {\n const [posts, setPosts] = useState([]);\n const [scrollPosition, setScrollPosition] = useState(0);\n\n useEffect(() => {\n setScrollPosition(0);\n }, [JSON.stringify(posts)]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Set state to literal when internal or external state changes", + "code": "\n function Game() {\n const [round, setRound] = useState(1);\n const [isGameOver, setIsGameOver] = useState(false);\n const { data: players } = useQuery('/players');\n\n useEffect(() => {\n if (round > 10 || players.length === 0) {\n setIsGameOver(true);\n }\n }, [round, players]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "Set state to external state when internal state changes", + "code": "\n function Game() {\n const [round, setRound] = useState(1);\n const [isGameOver, setIsGameOver] = useState(false);\n const { data: players } = useQuery('/players');\n\n useEffect(() => {\n if (round > 10) {\n setIsGameOver(players.length === 0);\n }\n }, [round, players]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "In an otherwise valid effect", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n const [otherState, setOtherState] = useState('Meow');\n\n useEffect(() => {\n console.log('Meow');\n setState('Hello World');\n }, [otherState]);\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-derived-state.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-derived-state.json new file mode 100644 index 000000000..bd9ba9f45 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-derived-state.json @@ -0,0 +1,351 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Compute in render from internal state", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Taylor');\n const [lastName, setLastName] = useState('Swift');\n\n const fullName = firstName + ' ' + lastName;\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Compute in render from props", + "code": "\n function Form({ firstName, lastName }) {\n const fullName = firstName + ' ' + lastName;\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Set to literal on external state change", + "code": "\n function Feed() {\n const { data: posts } = useQuery('/posts');\n const [scrollPosition, setScrollPosition] = useState(0);\n\n useEffect(() => {\n setScrollPosition(0);\n }, [posts]);\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "Set to derived literal on external state change", + "code": "\n function Feed() {\n const { data: posts } = useQuery('/posts');\n const [scrollPosition, setScrollPosition] = useState(0);\n\n useEffect(() => {\n const initialPosition = 0;\n setScrollPosition(initialPosition);\n }, [posts]);\n }\n ", + "todo": false + }, + { + "idx": 4, + "name": "Set to external state, with multiple setter calls", + "code": "\n function Feed() {\n const { data: posts } = useQuery('/posts');\n const [selectedPost, setSelectedPost] = useState();\n\n useEffect(() => {\n setSelectedPost(posts[0]);\n }, [posts]);\n\n return (\n
\n {posts.map((post) => (\n
setSelectedPost(post)}>\n {post.title}\n
\n ))}\n
\n )\n }\n ", + "todo": false + }, + { + "idx": 5, + "name": "Fetch external state on mount", + "code": "\n function Todos() {\n const [todos, setTodos] = useState([]);\n\n useEffect(() => {\n fetch('/todos').then((todos) => {\n setTodos(todos);\n });\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 6, + "name": "Sync external state", + "code": "\n function Search() {\n const [query, setQuery] = useState();\n const [results, setResults] = useState();\n\n useEffect(() => {\n fetch('/search?query=' + query).then((data) => {\n setResults(data);\n });\n }, [query]);\n\n return (\n
\n setQuery(e.target.value)}\n />\n
    \n {results.map((result) => (\n
  • {result.title}
    • \n ))}\n
\n
\n )\n }\n ", + "todo": false + }, + { + "idx": 7, + "name": "Synchronize internal state", + "code": "\n function Component() {\n const [name, setName] = useState();\n const [model] = useState(\n () => new FormModel(props)\n );\n\n useEffect(() => {\n model.setFieldDescriptor(name);\n return () => model.removeField(name);\n }, [model, name]);\n }\n ", + "todo": false + }, + { + "idx": 8, + "name": "Subscribe to external state", + "code": "\n import { subscribeToStatus } from 'library';\n\n function Status({ topic }) {\n const [status, setStatus] = useState();\n\n useEffect(() => {\n const unsubscribe = subscribeToStatus(topic, (status) => {\n setStatus(status);\n });\n\n return () => unsubscribe();\n }, [topic]);\n\n return
{status}
;\n }\n ", + "todo": false + }, + { + "idx": 9, + "name": "From derived external state with multiple calls to setter", + "code": "\n function Form() {\n const name = useQuery('/name');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n const prefixedName = 'Dr. ' + name;\n setFullName(prefixedName) \n }, [name]);\n\n return (\n setFullName(e.target.value)}\n />\n )\n }\n ", + "todo": false + }, + { + "idx": 10, + "name": "From props via unpure derived setter", + "code": "\n function DoubleCounter({ count }) {\n const [doubleCount, setDoubleCount] = useState(0);\n\n const derivedSetter = () => {\n const multipler = fetch('/multipler');\n setDoubleCount(multiplier);\n }\n\n useEffect(() => {\n derivedSetter();\n }, [count]);\n }\n ", + "todo": false + }, + { + "idx": 11, + "name": "Via unpure promise global function", + "code": "\n function Counter({ count }) {\n const [multipliedCount, setMultipliedCount] = useState();\n\n useEffect(() => {\n fetch('/multiplier')\n .then((res) => res.json())\n .then((multiplier) => setMultipliedCount(count * multiplier));\n }, [count]);\n\n return (\n // So single-setter doesn't trigger\n \n )\n }\n ", + "todo": false + }, + { + "idx": 12, + "name": "Defined-then-called async external global function", + "code": "\n function Component() {\n const api = useFetchWrapper();\n const [state, setState] = useState();\n\n useEffect(() => {\n async function fetchIt() {\n const response = await fetch('/endpoint');\n setState(response);\n }\n\n void fetchIt();\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 13, + "name": "Defined-then-called async function from API in deps", + "code": "\n function Component() {\n const api = useFetchWrapper();\n const [state, setState] = useState();\n\n useEffect(() => {\n async function fetchIt() {\n const response = await api.doFetch('/endpoint');\n setState(response);\n }\n\n void fetchIt();\n }, [api]);\n }\n ", + "todo": false + }, + { + "idx": 14, + "name": "From external data retrieved in async IIFE with API in deps", + "code": "\n function Component() {\n const api = useFetchWrapper();\n const [state, setState] = useState();\n\n useEffect(() => {\n (async function fetchIt() {\n const response = await api.doFetch('/endpoint');\n setState(response);\n })();\n }, [api]);\n }\n ", + "todo": false + }, + { + "idx": 15, + "name": "From external data retrieved in overly-complicated async IIFE", + "code": "\n import { useEffect, useState } from 'react';\n\n export const App = () => {\n const [response, setResponse] = useState(null);\n\n const fetchYesNoApi = () => {\n return (async () => {\n try {\n const response = await fetch('https://yesno.wtf/api');\n if (!response.ok) {\n throw new Error('Network error');\n }\n const data = await response.json();\n setResponse(data);\n } catch (err) {\n console.error(err);\n }\n })();\n };\n\n useEffect(() => { \n (async () => {\n await fetchYesNoApi();\n })();\n }, []);\n\n return (\n
{response}
\n );\n };\n ", + "todo": false + }, + { + "idx": 16, + "name": "Named function passed to event callback", + "code": "\n function Component() {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => {\n function handleClick() {\n setDoubleCount(count * 2);\n }\n\n document.addEventListener('click', handleClick);\n return () => document.removeEventListener('click', handleClick);\n }, [count]);\n }\n ", + "todo": false + }, + { + "idx": 17, + "name": "Pass internal args to external local function", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n const doSet = (arg1, arg2) => {\n console.log(arg1, arg2);\n }\n\n useEffect(() => {\n doSet(firstName, lastName);\n }, [firstName, lastName]);\n }\n ", + "todo": false + }, + { + "idx": 18, + "name": "Mutate internal state", + "code": "\n function DoubleList() {\n const [list, setList] = useState([]);\n const [doubleList, setDoubleList] = useState([]);\n\n useEffect(() => {\n doubleList.push(...list);\n }, [list]);\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "From internal state", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Taylor');\n const [lastName, setLastName] = useState('Swift');\n\n const [fullName, setFullName] = useState('');\n useEffect(() => setFullName(firstName + ' ' + lastName), [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "From derived internal state", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Taylor');\n const [lastName, setLastName] = useState('Swift');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n const name = firstName + ' ' + lastName;\n setFullName(name) \n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "From derived internal state outside effect", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Taylor');\n const [lastName, setLastName] = useState('Swift');\n const [fullName, setFullName] = useState('');\n const name = firstName + ' ' + lastName;\n\n useEffect(() => {\n setFullName(name) \n }, [name]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "From internal state via pure global function", + "code": "\n function Counter({ count }) {\n const [countJson, setCountJson] = useState();\n\n useEffect(() => {\n setCountJson(JSON.stringify(count));\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "From internal state via local unpure function", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n function computeName(firstName, lastName) {\n console.log('meow');\n return firstName + ' ' + lastName;\n }\n\n useEffect(() => {\n setFullName(computeName(firstName, lastName));\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "From internal state and external state", + "code": "\n import { usePrefix } from 'library';\n\n function Component() {\n const [name, setName] = useState();\n const [prefixedName, setPrefixedName] = useState();\n const prefix = usePrefix(name);\n\n useEffect(() => {\n const newValue = prefix + name;\n setPrefixedName(newValue);\n }, [prefix, name])\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 6, + "name": "From internal state via external function", + "code": "\n import { computeName } from 'library';\n\n function Component() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n setFullName(computeName(firstName, lastName));\n }, [firstName, lastName])\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 7, + "name": "From props", + "code": "\n function Form({ firstName, lastName }) {\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n setFullName(firstName + ' ' + lastName);\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 8, + "name": "From intermediate prop", + "code": "\n function Form({ firstName, lastName }) {\n const [fullName, setFullName] = useState('');\n const prefixedName = 'Dr. ' + firstName;\n\n useEffect(() => {\n setFullName(prefixedName + ' ' + lastName);\n }, [prefixedName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 9, + "name": "From props via method", + "code": "\n function DoubleList({ list }) {\n const [doubleList, setDoubleList] = useState([]);\n\n useEffect(() => {\n setDoubleList(list.concat(list));\n }, [list]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 10, + "name": "From internal state via method", + "code": "\n function DoubleList() {\n const [list, setList] = useState([]);\n const [doubleList, setDoubleList] = useState([]);\n\n useEffect(() => {\n setDoubleList(list.concat(list));\n }, [list]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 11, + "name": "From props and internal state", + "code": "\n function Form({ title }) {\n const [name, setName] = useState('Dwayne');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n setFullName(title + ' ' + name);\n }, [title, name]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 12, + "name": "From props and internal state via intermediate variable", + "code": "\n function Form({ title }) {\n const [name, setName] = useState('Dwayne');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n const newFullName = title + ' ' + name;\n setFullName(newFullName);\n }, [title, name]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 13, + "name": "From external state with single setter call", + "code": "\n function Feed() {\n const { data: posts } = fetch('/posts');\n const [selectedPost, setSelectedPost] = useState();\n\n useEffect(() => {\n setSelectedPost(posts[0]);\n }, [posts, setSelectedPost]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 14, + "name": "From internal plus external state with single setter call", + "code": "\n function Form() {\n const prefix = useQuery('/prefix');\n const [name, setName] = useState();\n const [prefixedName, setPrefixedName] = useState();\n\n useEffect(() => {\n setPrefixedName(prefix + name)\n }, [prefix, name]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 15, + "name": "From HOC prop with single setter call", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = withRouter(({ history }) => {\n const [location, setLocation] = useState();\n\n useEffect(() => {\n setLocation(history.location);\n }, [history.location]);\n });\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 16, + "name": "From props via callback setter", + "code": "\n import { useState, useEffect } from 'react';\n\n function CountAccumulator({ count }) {\n const [total, setTotal] = useState(count);\n\n useEffect(() => {\n setTotal((prev) => prev + count);\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 17, + "name": "From props via pure derived setter", + "code": "\n function DoubleCounter({ count }) {\n const [doubleCount, setDoubleCount] = useState(0);\n\n const derivedSetter = (count) => setDoubleCount(count * 2);\n\n useEffect(() => {\n derivedSetter(count);\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 18, + "name": "Partially update complex state from props", + "code": "\n function Form({ firstName, lastName }) {\n const [formData, setFormData] = useState({\n title: 'Dr.',\n fullName: '',\n });\n\n useEffect(() => {\n setFormData({\n ...formData,\n fullName: firstName + ' ' + lastName,\n });\n }, [firstName, lastName, formData]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 19, + "name": "Partially update complex state from props via callback setter", + "code": "\n function Form({ firstName, lastName }) {\n const [formData, setFormData] = useState({\n title: 'Dr.',\n fullName: '',\n });\n\n useEffect(() => {\n setFormData((prev) => ({\n ...prev,\n fullName: firstName + ' ' + lastName,\n }));\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 20, + "name": "Partially update complex state from props via derived setter", + "code": "\n function Form({ firstName, lastName }) {\n const [formData, setFormData] = useState({\n title: 'Dr.',\n fullName: '',\n });\n\n const setFullName = (fullName) => setFormData({ ...formData, fullName });\n\n useEffect(() => {\n setFormData({\n ...formData,\n fullName: firstName + ' ' + lastName,\n });\n }, [firstName, lastName, formData]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 21, + "name": "Derived state in larger, otherwise legit effect", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Taylor');\n const [lastName, setLastName] = useState('Swift');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n console.log(name);\n\n setFullName(firstName + ' ' + lastName);\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 22, + "name": "Set to result of pure local ArrowFunctionExpression", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n const computeName = (firstName, lastName) => {\n return firstName + ' ' + lastName;\n }\n\n useEffect(() => {\n setFullName(computeName(firstName, lastName));\n }, [firstName, lastName, computeName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 23, + "name": "Set to result of pure local FunctionDeclaration", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n function computeName(firstName, lastName) {\n return firstName + ' ' + lastName;\n }\n\n useEffect(() => {\n setFullName(computeName(firstName, lastName));\n }, [firstName, lastName, computeName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 24, + "name": "Set to result of semi-pure local ArrowFunctionExpression", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n const computeName = () => firstName + ' ' + lastName;\n\n setFullName(computeName());\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 25, + "name": "Set to result of semi-pure local FunctionDeclaration", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n function computeName() {\n return firstName + ' ' + lastName;\n }\n\n setFullName(computeName());\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 26, + "name": "Set to result of semi-pure function defined outside effect", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n const computeName = () => firstName + ' ' + lastName;\n\n useEffect(() => {\n setFullName(computeName());\n }, [computeName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 27, + "name": "Set to result of internal useCallback; repeat references to a useState variable", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n const computeName = useCallback(() => firstName + ' ' + lastName, [firstName, lastName]);\n\n useEffect(() => {\n setFullName(computeName());\n }, [computeName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 28, + "name": "Via no-arg intermediate setter", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n useEffect(() => {\n const doSet = () => {\n setFullName(firstName + ' ' + lastName);\n }\n\n doSet();\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 29, + "name": "From internal state via useCallback no-arg two-dep intermediate setter", + "code": "\n function Component() {\n const [name, setName] = useState();\n const [fullName, setFullName] = useState();\n const prefix = 'Dr.'\n\n const intermediateSetter = useCallback(() => {\n setFullName(prefix + ' ' + name);\n }, [prefix, name]);\n\n useEffect(() => {\n intermediateSetter();\n }, [intermediateSetter]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 30, + "name": "From internal state via useCallback one-arg one-dep intermediate setter", + "code": "\n function Component() {\n const [name, setName] = useState();\n const [fullName, setFullName] = useState();\n const prefix = 'Dr.'\n\n const intermediateSetter = useCallback((name) => {\n setFullName(prefix + ' ' + name);\n }, [prefix]);\n\n useEffect(() => {\n intermediateSetter(name);\n }, [name, intermediateSetter]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 31, + "name": "From internal state via useCallback two-arg no-dep intermediate setter", + "code": "\n function Component() {\n const [name, setName] = useState();\n const [fullName, setFullName] = useState();\n const prefix = 'Dr.'\n\n const intermediateSetter = useCallback((prefix, name) => {\n setFullName(prefix + ' ' + name);\n }, []);\n\n useEffect(() => {\n intermediateSetter(prefix, name);\n }, [prefix, name, intermediateSetter]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 32, + "name": "Pass state to derived setter which ignores args", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Dwayne');\n const [lastName, setLastName] = useState('The Rock');\n const [fullName, setFullName] = useState('');\n\n const setDerivedFullName = (firstName, lastName) => {\n setFullName(\"Sparky\");\n }\n\n useEffect(() => {\n setDerivedFullName(firstName, lastName);\n }, [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-event-handler.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-event-handler.json new file mode 100644 index 000000000..befb7d8c6 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-event-handler.json @@ -0,0 +1,75 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Sychronizing with external system", + "code": "\n function Search() {\n const [query, setQuery] = useState();\n const [results, setResults] = useState();\n\n useEffect(() => {\n fetch('/search?query=' + query).then((data) => {\n setResults(data);\n });\n }, [query]);\n\n return (\n
\n setQuery(e.target.value)}\n />\n
    \n {results.map((result) => (\n
  • {result.title}
    • \n ))}\n
\n
\n )\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Using props to handle an event and call an external function", + "code": "\n function Form({ dataToSubmit }) {\n useEffect(() => {\n if (dataToSubmit) {\n submitData(dataToSubmit);\n }\n }, [dataToSubmit]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Using state to handle an event and call an external function", + "code": "\n function Form() {\n const [name, setName] = useState();\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (dataToSubmit) {\n submitData(dataToSubmit);\n }\n }, [dataToSubmit]);\n\n return (\n
\n setName(e.target.value)}\n />\n \n
\n )\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "Using state to handle an event, no deps argument", + "code": "\n function Form() {\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (dataToSubmit) {\n submitData(dataToSubmit);\n }\n });\n\n return (\n \n )\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Using state to handle an event, empty deps", + "code": "\n function Form() {\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (dataToSubmit) {\n submitData(dataToSubmit);\n }\n }, []);\n\n return (\n \n )\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "Using state to handle an event and call a prop", + "code": "\n function Form({ submitData }) {\n const [name, setName] = useState();\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (dataToSubmit) {\n submitData(dataToSubmit);\n }\n }, [dataToSubmit]);\n\n return (\n
\n setName(e.target.value)}\n />\n \n
\n )\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "Early return in if test", + "code": "\n function Form() {\n const [name, setName] = useState();\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (!dataToSubmit) return;\n\n submitData(dataToSubmit);\n }, [dataToSubmit]);\n\n return (\n
\n setName(e.target.value)}\n />\n \n
\n )\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 6, + "name": "Member access and double test in condition", + "code": "\n function Form() {\n const [name, setName] = useState();\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (dataToSubmit.name && dataToSubmit.name.length > 0) {\n submitData(dataToSubmit);\n }\n }, [dataToSubmit]);\n\n return (\n
\n setName(e.target.value)}\n />\n \n
\n )\n }\n ", + "todo": false, + "errors": 2 + }, + { + "idx": 7, + "name": "If test includes non-state", + "code": "\n function Form() {\n const [name, setName] = useState();\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (dataToSubmit && Date.now() % 2 === 0) {\n submitData(dataToSubmit);\n }\n }, [dataToSubmit]);\n\n return (\n
\n setName(e.target.value)}\n />\n \n
\n )\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 8, + "name": "Klarna", + "code": "\n function Klarna({ klarnaAppId }) {\n const [countryCode] = useState(qs.parse('countryCode=meow'));\n const [result, setResult] = useState();\n const klarnaEnabled = useSelector('idk') && shouldKlarnaBeEnabled(countryCode);\n const currentLocale = getCurrentLocale(useGetCurrentLanguage());\n\n const loadSignInWithKlarna = (klarnaAppId, klarnaEnvironment, countryCode, currentLocale) => {\n const klarnaResult = doSomething();\n setResult(klarnaResult);\n };\n\n useEffect(() => {\n if (klarnaEnabled) {\n return loadSignInWithKlarna(\n klarnaAppId,\n klarnaEnvironment,\n countryCode?.toUpperCase(),\n currentLocale,\n );\n }\n }, [\n countryCode,\n klarnaAppId,\n klarnaEnabled,\n klarnaEnvironment,\n currentLocale,\n ]);\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-initialize-state.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-initialize-state.json new file mode 100644 index 000000000..b32c0f088 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-initialize-state.json @@ -0,0 +1,84 @@ +{ + "valid": [ + { + "idx": 0, + "name": "To external data via Promise", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n\n useEffect(() => {\n fetch(\"https://api.example.com/data\")\n .then(response => response.json())\n .then(data => setState(data));\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "To external data via async IIFE", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n\n useEffect(() => {\n (async () => {\n const response = await fetch(\"https://api.example.com/data\");\n const data = await response.json();\n setState(data);\n })();\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "To literal inside synchronous IIFE", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n\n useEffect(() => {\n (() => {\n setState(\"Hello\");\n })();\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "To literal inside IIFE inside callback", + "code": "\n import { useEffect, useState } from 'react';\n\n export const MyComponent = () => {\n const [state, setState] = useState();\n\n useEffect(() => {\n window.addEventListener('load', () => {\n (() => {\n setState('Loaded');\n })();\n });\n }, []);\n };\n ", + "todo": false + }, + { + "idx": 4, + "name": "To literal inside callback inside IIFE", + "code": "\n import { useEffect, useState } from 'react';\n\n export const MyComponent = () => {\n const [state, setState] = useState();\n\n useEffect(() => {\n (() => {\n window.addEventListener('load', () => {\n setState('Loaded');\n });\n })();\n }, []);\n };\n ", + "todo": false + }, + { + "idx": 5, + "name": "Setter with other dependency", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n const [other, setOther] = useState();\n\n useEffect(() => {\n setState(\"Hello\");\n }, [other]);\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "To literal", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n\n useEffect(() => {\n setState(\"Hello\");\n }, []);\n\n return
{state}
;\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "To internal data", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n const [otherState, setOtherState] = useState('Meow');\n\n useEffect(() => {\n setState(otherState);\n }, []);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "In an otherwise valid effect", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n\n useEffect(() => {\n console.log('Meow');\n setState('Hello World');\n }, []);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "To undefined", + "code": "\n function MyComponent() {\n const [state, setState] = useState('Meow');\n\n useEffect(() => {\n setState();\n }, []);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "To equivalent derived literals", + "code": "\n function MyComponent() {\n const upstream = 'Meow';\n const [state, setState] = useState(upstream);\n\n useEffect(() => {\n const upstreamTwo = 'Meow';\n setState(upstreamTwo);\n }, []);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "Only setter in deps", + "code": "\n function MyComponent() {\n const [state, setState] = useState();\n\n useEffect(() => {\n setState(\"Hello\");\n }, [setState]);\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-pass-data-to-parent.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-pass-data-to-parent.json new file mode 100644 index 000000000..32ccf2c56 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-pass-data-to-parent.json @@ -0,0 +1,137 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Pass literal value", + "code": "\n const Child = ({ onTextChanged }) => {\n useEffect(() => {\n onTextChanged(\"Hello World\");\n }, [onTextChanged]);\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Pass derived literal value", + "code": "\n const Child = ({ onTextChanged }) => {\n const hello = \"Hello\";\n const world = \"World\";\n const greeting = hello + \" \" + world;\n useEffect(() => {\n onTextChanged(greeting);\n }, [onTextChanged]);\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Pass internal state", + "code": "\n const Child = ({ onTextChanged }) => {\n const [text, setText] = useState();\n\n useEffect(() => {\n onTextChanged(text);\n }, [onTextChanged, text]);\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "Pass prop", + "code": "\n const Child = ({ text, onTextChanged }) => {\n useEffect(() => {\n onTextChanged(text);\n }, [onTextChanged, text]);\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false + }, + { + "idx": 4, + "name": "No-arg prop callback", + "code": "\n function Form({ onClose }) {\n const [name, setName] = useState();\n const [isOpen, setIsOpen] = useState(true);\n\n useEffect(() => {\n if (!isOpen) {\n onClose();\n }\n }, [isOpen]);\n\n return (\n \n )\n }\n ", + "todo": false + }, + { + "idx": 5, + "name": "Prop getter", + "code": "\n function Child({ getData }) {\n useEffect(() => {\n console.log(getData());\n }, [getData]);\n }\n ", + "todo": false + }, + { + "idx": 6, + "name": "Pass internal state to HOC prop", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = withRouter(({ history }) => {\n const [option, setOption] = useState();\n\n useEffect(() => {\n history.push(option);\n }, [option]);\n });\n ", + "todo": false + }, + { + "idx": 7, + "name": "Pass external state to HOC prop", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = withRouter(({ history }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n if (data.error) {\n history.push('/error');\n }\n }, [data]);\n });\n ", + "todo": false + }, + { + "idx": 8, + "name": "Pass ref to parent", + "code": "\n const Child = ({ onRef }) => {\n const ref = useRef();\n\n useEffect(() => {\n onRef(ref.current);\n }, [onRef, ref.current]);\n }\n ", + "todo": false + }, + { + "idx": 9, + "name": "Pass cleanup function that depends on ref", + "code": "\n import { dropTargetForElements } from \"@atlaskit/pragmatic-drag-and-drop/element/adapter\";\n\n function DeleteDropTarget({ onDelete }) {\n const ref = React.useRef(null);\n\n React.useEffect(() => {\n const element = ref.current;\n if (!element) {\n return;\n }\n\n const cleanup = dropTargetForElements({\n element,\n onDrop: ({ source }) => {\n onDelete(source.data);\n },\n });\n\n return cleanup;\n }, [onDelete]);\n\n return
Drop an item here to delete
;\n };\n ", + "todo": false + }, + { + "idx": 10, + "name": "Effect inside custom hook returns MemberExpression cleanup", + "code": "\n function useActorLogger(actorRef) {\n useEffect(() => {\n return actorRef.system.inspect((next) => {\n if (next.type === '@xstate.snapshot') {\n console.log('ACTOR SNAPSHOT', next.snapshot);\n }\n }).unsubscribe;\n }, [actorRef]);\n }\n ", + "todo": false + }, + { + "idx": 11, + "name": "Register callback on own ref to pass data to parent", + "code": "\n const Child = ({ onClicked }) => {\n const ref = useRef();\n\n useEffect(() => {\n ref.current.addEventListener('click', (event) => {\n onClicked(event);\n });\n }, [onClicked, ref]);\n }\n ", + "todo": false + }, + { + "idx": 12, + "name": "Register external callback on ref prop", + "code": "\n const Child = ({ ref }) => {\n useEffect(() => {\n ref.current.addEventListener('click', (event) => {\n console.log('Clicked', event);\n });\n }, [ref]);\n }\n ", + "todo": false + }, + { + "idx": 13, + "name": "Pass external state that's retrieved in effect via .then", + "code": "\n const Child = ({ onFetched }) => {\n useEffect(() => {\n fetchData()\n .then((data) => onFetched(data));\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 14, + "name": "Pass external state that's retrieved in effect via async/await", + "code": "\n const Child = ({ onFetched }) => {\n useEffect(() => {\n (async () => {\n const data = await fetchData();\n onFetched(data);\n })();\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 15, + "name": "Pass window event data to parent", + "code": "\n const Child = ({ onResized }) => {\n useEffect(() => {\n window.addEventListener('resize', (event) => {\n onResized({\n width: window.innerWidth,\n height: window.innerHeight,\n });\n });\n return () => window.removeEventListener('resize', handleResize);\n }, [onResized]);\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Pass external state", + "code": "\n const Child = ({ onFetched }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n onFetched(data);\n }, [onFetched, data]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Pass external state, no deps argument", + "code": "\n const Child = ({ onFetched }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n onFetched(data);\n });\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "Pass external state, empty deps", + "code": "\n const Child = ({ onFetched }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n onFetched(data);\n }, []);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Pass external state in custom hook", + "code": "\n const useCustomHook = ({ onFetched }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n onFetched(data);\n }, [onFetched, data]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "Pass derived external state", + "code": "\n const Child = ({ onFetched }) => {\n const data = useSomeAPI();\n const firstElement = data[0];\n\n useEffect(() => {\n onFetched(firstElement);\n }, [onFetched, firstElement]);\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-pass-live-state-to-parent.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-pass-live-state-to-parent.json new file mode 100644 index 000000000..85cd84523 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-pass-live-state-to-parent.json @@ -0,0 +1,135 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Pass literal value to prop callback", + "code": "\n const Child = ({ onTextChanged }) => {\n useEffect(() => {\n onTextChanged(\"Hello World\");\n }, [onTextChanged]);\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Pass live external state", + "code": "\n const Child = ({ onFetched }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n onFetched(data);\n }, [onFetched, data]);\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Pass prop to parent", + "code": "\n const Child = ({ text, onTextChanged }) => {\n useEffect(() => {\n onTextChanged(text);\n }, [text, onTextChanged]);\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "No-arg prop callback", + "code": "\n function Form({ onClose }) {\n const [name, setName] = useState();\n const [isOpen, setIsOpen] = useState(true);\n\n useEffect(() => {\n if (!isOpen) {\n onClose();\n }\n }, [isOpen]);\n\n return (\n \n )\n }\n ", + "todo": false + }, + { + "idx": 4, + "name": "Prop getter", + "code": "\n function Child({ getData }) {\n useEffect(() => {\n console.log(getData());\n }, [getData]);\n }\n ", + "todo": false + }, + { + "idx": 5, + "name": "Pass internal state to HOC prop", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = withRouter(({ history }) => {\n const [option, setOption] = useState();\n\n useEffect(() => {\n history.push(option);\n }, [option]);\n });\n ", + "todo": false + }, + { + "idx": 6, + "name": "Pass internal state to weird HOC prop", + "code": "\n const MyComponent = inject('ourStore')(observer(({ ourStore }) => {\n const [option, setOption] = useState();\n\n useEffect(() => {\n ourStore.push(option);\n }, [option]);\n }));\n ", + "todo": false + }, + { + "idx": 7, + "name": "Pass internal state to separately-wrapped HOC prop", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = ({ history }) => {\n const [option, setOption] = useState();\n\n useEffect(() => {\n history.push(option);\n }, [option]);\n };\n\n const wrapped = withRouter(MyComponent);\n ", + "todo": false + }, + { + "idx": 8, + "name": "Pass internal state to weirdly-separately-wrapped HOC prop", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = ({ history }) => {\n const [option, setOption] = useState();\n\n useEffect(() => {\n history.push(option);\n }, [option]);\n };\n\n const EnhancedComponent = inject('ourStore')(observer(MyComponent))\n export default EnhancedComponent\n ", + "todo": false + }, + { + "idx": 9, + "name": "Pass external state to HOC prop", + "code": "\n import { withRouter } from 'react-router-dom';\n\n const MyComponent = withRouter(({ history }) => {\n const data = useSomeAPI();\n\n useEffect(() => {\n if (data.error) {\n history.push(data.error);\n }\n }, [data]);\n });\n ", + "todo": false + }, + { + "idx": 10, + "name": "Pass ref to parent", + "code": "\n const Child = ({ onRef }) => {\n const ref = useRef();\n\n useEffect(() => {\n onRef(ref.current);\n }, [onRef, ref.current]);\n\n return
Child
;\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Pass live internal state", + "code": "\n const Child = ({ onTextChanged }) => {\n const [text, setText] = useState();\n\n useEffect(() => {\n onTextChanged(text);\n }, [onTextChanged, text]);\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Pass live internal state, no deps argument", + "code": "\n const Child = ({ onTextChanged }) => {\n const [text, setText] = useState();\n\n useEffect(() => {\n onTextChanged(text);\n });\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "Pass live internal state, empty deps", + "code": "\n const Child = ({ onTextChanged }) => {\n const [text, setText] = useState();\n\n useEffect(() => {\n onTextChanged(text);\n }, []);\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Pass live derived internal state in custom hook", + "code": "\n const useCustomHook = ({ onTextChanged }) => {\n const [text, setText] = useState();\n\n useEffect(() => {\n onTextChanged(text);\n }, [onTextChanged, text]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "Pass live internal state AND external state", + "code": "\n const Child = ({ onTextChanged }) => {\n const [text, setText] = useState();\n const data = useSomeAPI();\n\n useEffect(() => {\n onTextChanged(text, data);\n }, [onTextChanged, text, data]);\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "Pass live derived internal state", + "code": "\n const Child = ({ onTextChanged }) => {\n const [text, setText] = useState();\n\n useEffect(() => {\n const firstChar = text[0];\n onTextChanged(firstChar);\n }, [onTextChanged, text]);\n\n return (\n setText(e.target.value)}\n />\n );\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 6, + "name": "Pass live internal state via derived prop", + "code": "\n const Child = ({ onFetched }) => {\n const [data, setData] = useState();\n const onFetchedWrapper = (v) => onFetched(v);\n\n useEffect(() => {\n onFetchedWrapper(data);\n }, [onFetched, data]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 7, + "name": "Pass live internal state via later-destructured prop", + "code": "\n const Child = (props) => {\n const [data, setData] = useState();\n const { onFetched } = props;\n\n useEffect(() => {\n onFetched(data);\n }, [onFetched, data]);\n }\n ", + "todo": true, + "errors": 1 + }, + { + "idx": 8, + "name": "Pass final internal state", + "code": "\n function Form({ onSubmit }) {\n const [name, setName] = useState();\n const [dataToSubmit, setDataToSubmit] = useState();\n\n useEffect(() => {\n if (!dataToSubmit) return;\n\n onSubmit(dataToSubmit);\n }, [dataToSubmit]);\n\n return (\n
\n setName(e.target.value)}\n />\n \n
\n )\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-reset-all-state-on-prop-change.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-reset-all-state-on-prop-change.json new file mode 100644 index 000000000..28a7225e7 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/no-reset-all-state-on-prop-change.json @@ -0,0 +1,103 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Set state when a prop changes, but not to its default value", + "code": "\n function List({ items }) {\n const [selection, setSelection] = useState();\n\n useEffect(() => {\n setSelection(items[0]);\n }, [items]);\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Reset some state when a prop changes", + "code": "\n function ProfilePage({ userId }) {\n const [user, setUser] = useState(null);\n const [comment, setComment] = useState('type something');\n const [catName, setCatName] = useState('Sparky');\n\n useEffect(() => {\n setUser(null);\n setComment('meow')\n }, [userId]);\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Undefined state initializer compared to state setter with literal null", + "code": "\n function List({ items }) {\n const [selectedItem, setSelectedItem] = useState();\n\n useEffect(() => {\n setSelectedItem(null);\n }, [items]);\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "Reset all state when a prop changes inside lowercased function definition", + "code": "\n function buildComponent() {\n const [comment, setComment] = useState('type something');\n\n useEffect(() => {\n setComment('type something');\n }, [userId]);\n\n return
hi
;\n }\n ", + "todo": false + }, + { + "idx": 4, + "name": "Reset all state when a prop changes in a custom hook", + "code": "\n function useCustomHook({ userId }) {\n const [user, setUser] = useState(null);\n const [comment, setComment] = useState('type something');\n\n useEffect(() => {\n setUser(null);\n setComment('type something');\n }, [userId]);\n }\n ", + "todo": false + }, + { + "idx": 5, + "name": "Reset all state to derived initial state when a prop changes", + "code": "\n function ProfilePage({ userId }) {\n const initialState = 'meow meow'\n const [comment, setComment] = useState(initialState);\n\n useEffect(() => {\n const derivedInitialState = initialState + '!';\n setComment(derivedInitialState);\n }, [userId]);\n }\n ", + "todo": false + }, + { + "idx": 6, + "name": "Set state differently in callback to state-like hook", + "code": "\n const Foo = () => {\n const [_0, setState] = useState(false);\n const [_1, startTransition] = useTransition();\n\n useEffect(() => {\n startTransition(() => {\n setState(true);\n });\n }, []);\n\n return null;\n };\n ", + "todo": false + }, + { + "idx": 7, + "name": "Set state differently in callback to state-like hook", + "code": "\n const Foo = () => {\n const [_0, setState] = React.useState(false);\n const [_1, startTransition] = React.useTransition();\n\n useEffect(() => {\n startTransition(() => {\n setState(true);\n });\n }, []);\n\n return null;\n };\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Reset all state when a prop changes", + "code": "\n function ProfilePage({ userId }) {\n const [user, setUser] = useState(null);\n const [comment, setComment] = useState('type something');\n\n useEffect(() => {\n setUser(null);\n setComment('type something');\n }, [userId]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Reset all state to shared var when a prop changes", + "code": "\n function ProfilePage({ userId }) {\n const initialState = 'meow meow'\n const [user, setUser] = useState(null);\n const [comment, setComment] = useState(initialState);\n\n useEffect(() => {\n setUser(null);\n setComment(initialState);\n }, [userId]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "Reset all state when a prop member changes", + "code": "\n function ProfilePage({ user }) {\n const [comment, setComment] = useState('type something');\n\n useEffect(() => {\n setComment('type something');\n }, [user.id]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Reset all state when one of two props change", + "code": "\n function ProfilePage({ userId, friends }) {\n const [comment, setComment] = useState('type something');\n\n useEffect(() => {\n setComment('type something');\n }, [userId, friends]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 4, + "name": "Undefined state initializer compared to state setter with literal undefined", + "code": "\n function List({ items }) {\n const [selectedItem, setSelectedItem] = useState();\n\n useEffect(() => {\n setSelectedItem(undefined);\n }, [items]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "Meow", + "code": "\n const ExternalAssetItemRow = memo(\n ({\n id,\n title,\n exportIdentifier,\n localId,\n hasUpdate,\n isViewOnly,\n getMenuOptions,\n onUpdate,\n onDragStart,\n Icon,\n exitMode,\n }) => {\n const [shouldUpdate, setShouldUpdate] = useState(hasUpdate);\n\n useEffect(() => {\n setShouldUpdate(hasUpdate);\n }, [hasUpdate]);\n\n const onClickUpdate = useCallback(\n (event) => {\n event.stopPropagation();\n\n if (isViewOnly) return;\n\n setShouldUpdate(false);\n },\n [onUpdate, exportIdentifier, title, isViewOnly],\n );\n\n const handleDragStart = useCallback(\n (event) => {\n exitMode();\n onDragStart(event, exportIdentifier);\n },\n [onDragStart, exportIdentifier],\n );\n\n const getMenu = useCallback(\n (id) => getMenuOptions(id, exportIdentifier, title, localId),\n [getMenuOptions, exportIdentifier, title, localId],\n );\n\n return (\n \n \n )\n },\n );\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 6, + "name": "Reset all state to function call result when a prop changes", + "code": "\n function ProfilePage({ userId }) {\n const [comment, setComment] = useState(getInitialComment());\n\n useEffect(() => {\n setComment(getInitialComment());\n }, [userId]);\n }\n\n function getInitialComment() {\n return 'type something';\n }\n ", + "todo": false, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/real-world.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/real-world.json new file mode 100644 index 000000000..1c839302c --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/real-world.json @@ -0,0 +1,107 @@ +{ + "valid": [ + { + "idx": 0, + "name": "useLayoutEffect", + "code": "\n function Input({ count }) {\n const ref = useRef();\n\n useLayoutEffect(() => {\n if (count == 0) {\n ref.current?.focus();\n }\n }, [count]);\n\n return (\n \n )\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Managing a timer", + "code": "\n function Timer() {\n const [seconds, setSeconds] = useState(0);\n\n useEffect(() => {\n const interval = setInterval(() => {\n setSeconds((s) => s + 1);\n }, 1000);\n\n return () => { \n clearInterval(interval); \n }\n }, []);\n\n return
{seconds}
;\n }\n ", + "todo": false + }, + { + "idx": 2, + "name": "Debouncing", + "code": "\n function useDebouncedState(value, delay) {\n const [state, setState] = useState(value);\n const [debouncedState, setDebouncedState] = useState(value);\n\n useEffect(() => {\n const timeout = setTimeout(() => {\n setDebouncedState(state);\n }, delay);\n\n return () => {\n clearTimeout(timeout);\n };\n }, [delay, state]);\n\n return [state, debouncedState, setState];\n }\n ", + "todo": false + }, + { + "idx": 3, + "name": "Debouncing via Lodash", + "code": "\n import { useState, useEffect } from 'react';\n import debounce from 'lodash/debounce';\n\n export const useDebouncedState = (delay) => {\n const [value] = useState(0);\n\n const debouncedFunction = debounce((newValue) => {\n console.log(newValue);\n }, delay);\n\n useEffect(() => {\n debouncedFunction(value);\n }, [value, debouncedFunction]);\n\n return [];\n };\n ", + "todo": false + }, + { + "idx": 4, + "name": "Listening for window events", + "code": "\n function WindowSize() {\n const [size, setSize] = useState({ width: window.innerWidth, height: window.innerHeight });\n\n useEffect(() => {\n const handleResize = () => {\n setSize({ width: window.innerWidth, height: window.innerHeight });\n };\n\n window.addEventListener('resize', handleResize);\n\n return () => {\n window.removeEventListener('resize', handleResize);\n };\n }, []);\n\n return
{size.width} x {size.height}
;\n }\n ", + "todo": false + }, + { + "idx": 5, + "name": "ResizeObserver", + "code": "\n function useHasOverflow({ contentRef, maxHeight }) {\n const [hasOverflow, setHasOverflow] = useState(false);\n\n useEffect(() => {\n const resizeObserver = new ResizeObserver((element) => {\n const hasContentOverflow = element.scrollHeight > maxHeight;\n setHasOverflow(hasContentOverflow);\n })\n\n if (contentRef.current != null) {\n resizeObserver.observe(contentRef.current);\n }\n\n return () => {\n resizeObserver.disconnect();\n };\n }, [contentRef, maxHeight]);\n\n return hasOverflow;\n }\n ", + "todo": false + }, + { + "idx": 6, + "name": "Play/pausing DOM video", + "code": "\n function VideoPlayer() {\n const [isPlaying, setIsPlaying] = useState(false);\n const videoRef = useRef();\n\n useEffect(() => {\n if (isPlaying) {\n videoRef.current.play();\n } else {\n videoRef.current.pause();\n }\n }, [isPlaying]);\n\n return
\n
\n }\n ", + "todo": false + }, + { + "idx": 7, + "name": "Saving to LocalStorage", + "code": "\n function Notes() {\n const [notes, setNotes] = useState(() => {\n const savedNotes = localStorage.getItem('notes');\n return savedNotes ? JSON.parse(savedNotes) : [];\n });\n\n useEffect(() => {\n localStorage.setItem('notes', JSON.stringify(notes));\n }, [notes]);\n\n return setNotes(e.target.value)}\n />\n }\n ", + "todo": false + }, + { + "idx": 8, + "name": "Logging/Analytics", + "code": "\n function Nav() {\n const [page, setPage] = useState('home');\n\n useEffect(() => {\n console.log(\"page viewed\", page);\n }, [page]);\n\n return (\n
\n \n \n
{page}
\n
\n )\n }\n ", + "todo": false + }, + { + "idx": 9, + "name": "CountryPicker", + "code": "\n function CountryPicker({ withEmoji }) {\n const { translation, getCountries } = useContext();\n\n const [state, setState] = useState({\n countries: [],\n selectedCountry: null,\n });\n const setCountries = (countries) => setState({ ...state, countries });\n\n useEffect(() => {\n let cancel = false;\n getCountries(translation)\n .then((countries) => (cancel ? null : setCountries(countries)))\n .catch(console.warn);\n\n return () => {\n cancel = true;\n };\n }, [translation, withEmoji]);\n }\n ", + "todo": false + }, + { + "idx": 10, + "name": "navigation.setOptions", + "code": "\n import { useNavigation } from '@react-navigation/native';\n import { useState, useLayoutEffect } from 'react';\n\n function ProfileScreen({ route }) {\n const navigation = useNavigation();\n const [value, onChangeText] = React.useState(route.params.title);\n\n React.useLayoutEffect(() => {\n navigation.setOptions({\n title: value === '' ? 'No title' : value,\n });\n }, [navigation, route]);\n }\n ", + "todo": false + }, + { + "idx": 11, + "name": "Keyboard state listener", + "code": "\n import { useEffect, useState } from 'react';\n import keyboardReducer from './reducers';\n\n let globalKeyboardState = {\n recentlyUsed: []\n };\n\n export const keyboardStateListeners = new Set();\n\n const setKeyboardState = (action) => {\n globalKeyboardState = keyboardReducer(globalKeyboardState, action);\n keyboardStateListeners.forEach((listener) => listener(globalKeyboardState));\n };\n\n export const useKeyboardStore = () => {\n const [keyboardState, setState] = useState(globalKeyboardState);\n\n useEffect(() => {\n const listener = () => setState(globalKeyboardState);\n keyboardStateListeners.add(listener);\n return () => {\n keyboardStateListeners.delete(listener);\n };\n }, [keyboardState]);\n\n return { keyboardState, setKeyboardState };\n };\n\n useKeyboardStore.setKeyboardState = setKeyboardState;\n ", + "todo": false + }, + { + "idx": 12, + "name": "Indexing ref state with internal state", + "code": "\n import { useEffect, useRef, useState } from \"react\";\n\n const someArray = [{ id: 1 }, { id: 2 }, { id: 3 }];\n\n const Component = ({ value }) => {\n const inputRefs = useRef([]);\n\n useEffect(() => {\n inputRefs.current?.[index]?.focus();\n }, [value, index]);\n\n return (\n <>\n {someArray.map((item, index) => (\n (inputRefs.current[index] = el)}\n />\n ))}\n \n )\n }\n ", + "todo": false + }, + { + "idx": 13, + "name": "Ref callback", + "code": "\n export const useOnScreen = () => {\n const [element, setElement] = useState(null);\n const [isIntersecting, setIntersecting] = useState(false);\n\n const ref = useCallback((element) => {\n setElement(element);\n }, []);\n\n useEffect(() => {\n if (!element) {\n return;\n }\n\n const observer = new IntersectionObserver(([entry]) => {\n setIntersecting(entry?.isIntersecting ?? false);\n });\n\n observer.observe(element);\n return () => {\n observer.disconnect();\n };\n }, [element]);\n\n return { ref, isIntersecting };\n };\n ", + "todo": false + }, + { + "idx": 14, + "name": "Effect with recursion", + "code": "\n function Component() {\n useEffect(() => {\n const container = ctnDom.current\n if (!container) return\n\n // WebGL setup (~30 lines)\n const renderer = new Renderer({ alpha: true, premultipliedAlpha: false })\n const gl = renderer.gl\n const program = new Program(gl, {\n vertex: vert,\n fragment: frag,\n uniforms: {\n iTime: { value: 0 },\n iResolution: { value: new Vec3(gl.canvas.width, gl.canvas.height, gl.canvas.width / gl.canvas.height) },\n hue: { value: hue },\n hover: { value: 0 },\n rot: { value: 0 },\n hoverIntensity: { value: hoverIntensity },\n },\n })\n const mesh = new Mesh(gl, { geometry, program })\n\n // Nested function 4: animation loop with recursion\n let rafId\n let lastTime = 0\n let currentRot = 0\n const rotationSpeed = 0.3\n\n const update = (t) => {\n rafId = requestAnimationFrame(update) // Recursive call\n const dt = (t - lastTime) * 0.001\n lastTime = t\n const currentTime = t * 0.001\n program.uniforms.iTime.value = currentTime\n\n // Color cycle\n if (cycleHue) {\n const cyclicHue = (hue + currentTime * hueCycleSpeed) % 360\n program.uniforms.hue.value = cyclicHue\n } else {\n program.uniforms.hue.value = hue\n }\n\n program.uniforms.hoverIntensity.value = hoverIntensity\n\n const effectiveHover = forceHoverState ? 1 : targetHover\n program.uniforms.hover.value += (effectiveHover - program.uniforms.hover.value) * 0.1\n\n if (rotateOnHover && effectiveHover > 0.5) {\n currentRot += dt * rotationSpeed\n }\n program.uniforms.rot.value = currentRot\n\n renderer.render({ scene: mesh })\n }\n rafId = requestAnimationFrame(update)\n\n // Cleanup function\n return () => {\n cancelAnimationFrame(rafId)\n window.removeEventListener(\"resize\", resize)\n container.removeEventListener(\"mousemove\", handleMouseMove)\n container.removeEventListener(\"mouseleave\", handleMouseLeave)\n container.removeChild(gl.canvas)\n gl.getExtension(\"WEBGL_lose_context\")?.loseContext()\n }\n }, [\n hue,\n hoverIntensity,\n rotateOnHover,\n forceHoverState,\n cycleHue,\n hueCycleSpeed,\n size,\n ])\n }\n ", + "todo": false + }, + { + "idx": 15, + "name": "TanStack useInfinityQuery useInView", + "code": "\n import React from 'react'\n import { useInView } from 'react-intersection-observer'\n import { useInfiniteQuery } from '@tanstack/react-query'\n\n function Example() {\n const { ref, inView } = useInView()\n\n const {\n status,\n data,\n error,\n isFetching,\n isFetchingNextPage,\n isFetchingPreviousPage,\n fetchNextPage,\n fetchPreviousPage,\n hasNextPage,\n hasPreviousPage,\n } = useInfiniteQuery({\n queryKey: ['projects'],\n queryFn: async ({\n pageParam,\n }) => {\n const response = await fetch('/api/projects?cursor=' + pageParam)\n return await response.json()\n },\n initialPageParam: 0,\n getPreviousPageParam: (firstPage) => firstPage.previousId,\n getNextPageParam: (lastPage) => lastPage.nextId,\n })\n\n React.useEffect(() => {\n if (inView && hasNextPage && !isFetchingNextPage) {\n fetchNextPage()\n }\n }, [inView, hasNextPage, isFetchingNextPage, fetchNextPage])\n }\n ", + "todo": false + }, + { + "idx": 16, + "name": "TanStack useInfinityQuery useInView with state, prop and data in queryKey", + "code": "\n import React from 'react'\n import { useInView } from 'react-intersection-observer'\n import { useInfiniteQuery } from '@tanstack/react-query'\n\n function Example(props) {\n const { ref, inView } = useInView()\n const [state] = React.useState(0)\n const search = useSearchParams()\n\n const {\n status,\n data,\n error,\n isFetching,\n isFetchingNextPage,\n isFetchingPreviousPage,\n fetchNextPage,\n fetchPreviousPage,\n hasNextPage,\n hasPreviousPage,\n } = useInfiniteQuery({\n // Makes the plugin interpret fetchNextPage as a state/prop/data function\n queryKey: ['projects', state, props, search],\n queryFn: async ({\n pageParam,\n }) => {\n const response = await fetch('/api/projects?cursor=' + pageParam)\n return await response.json()\n },\n initialPageParam: 0,\n getPreviousPageParam: (firstPage) => firstPage.previousId,\n getNextPageParam: (lastPage) => lastPage.nextId,\n })\n\n React.useEffect(() => {\n // TODO: Should maybe ideally also not flag no-event-handler here?\n // if (inView && hasNextPage && !isFetchingNextPage) {\n // Must have \"void\" so we can infer it's an async function\n void fetchNextPage()\n // }\n }, [inView, hasNextPage, isFetchingNextPage, fetchNextPage])\n }\n ", + "todo": false + } + ], + "invalid": [] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/syntax.json b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/syntax.json new file mode 100644 index 000000000..382f604d9 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-fixtures/syntax.json @@ -0,0 +1,221 @@ +{ + "valid": [ + { + "idx": 0, + "name": "Two components with overlapping names", + "code": "\n function ComponentOne() {\n const [data, setData] = useState();\n }\n\n function ComponentTwo() {\n const setData = (data) => {\n console.log(data);\n }\n\n useEffect(() => {\n setData('hello');\n }, []);\n }\n ", + "todo": false + }, + { + "idx": 1, + "name": "Reacting to external state changes with member access in deps", + "code": "\n function Feed() {\n const { data } = useQuery('/posts');\n const [scrollPosition, setScrollPosition] = useState(0);\n\n useEffect(() => {\n setScrollPosition(0);\n }, [data.posts]);\n }\n ", + "todo": false + } + ], + "invalid": [ + { + "idx": 0, + "name": "Derived state, with imports", + "code": "\n import { useState, useEffect } from 'react';\n\n function DoubleCounter() {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(count * 2), [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 1, + "name": "Derived state, without imports", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('Taylor');\n const [lastName, setLastName] = useState('Swift');\n\n const [fullName, setFullName] = useState('');\n useEffect(() => setFullName(firstName + ' ' + lastName), [firstName, lastName]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 2, + "name": "Derived state, React.useEffect import", + "code": "\n import * as React from 'react';\n\n function DoubleCounter() {\n const [count, setCount] = React.useState(0);\n const [doubleCount, setDoubleCount] = React.useState(0);\n\n React.useEffect(() => {\n setDoubleCount(count * 2);\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 3, + "name": "Derived state, with renamed import", + "code": "\n import { useState as stateUser, useEffect } from 'react';\n\n function Form() {\n const [firstName, setFirstName] = stateUser('Taylor');\n const [lastName, setLastName] = stateUser('Swift');\n\n const [fullName, setFullName] = stateUser('');\n useEffect(() => setFullName(firstName + ' ' + lastName), [firstName, lastName]);\n }\n ", + "todo": true, + "errors": 1 + }, + { + "idx": 4, + "name": "Function component", + "code": "\n function DoubleCounter() {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(count * 2), [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 5, + "name": "Arrow function component", + "code": "\n const DoubleCounter = () => {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(count * 2), [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 6, + "name": "Memoized component, with props", + "code": "\n const DoubleCounter = memo(({ count }) => {\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(count), [count]);\n });\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 7, + "name": "Effect one-liner body", + "code": "\n const AvoidDuplicateTest = () => {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(count * 2), [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 8, + "name": "Effect single-statement body", + "code": "\n const DoubleCounter = () => {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => { setDoubleCount(count * 2); }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 9, + "name": "Effect multi-statement body", + "code": "\n const DoubleCounter = () => {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => { setDoubleCount(count * 2); setDoubleCount(count * 2); }, [count]);\n }\n ", + "todo": false, + "errors": 2 + }, + { + "idx": 10, + "name": "Effect anonymous function body", + "code": "\n const DoubleCounter = () => {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(function() { setDoubleCount(count * 2); }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 11, + "name": "Non-destructured props", + "code": "\n function DoubleCounter(props) {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(props.count * 2), [props.count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 12, + "name": "Destructured props", + "code": "\n function DoubleCounter({ propCount }) {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(propCount * 2), [propCount]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 13, + "name": "Renamed destructured props", + "code": "\n function DoubleCounter({ count: countProp }) {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(countProp * 2), [countProp]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 14, + "name": "Doubly deep MemberExpression in effect", + "code": "\n function DoubleCounter(props) {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(props.nested.count * 2), [props.nested.count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 15, + "name": "Objects stored in state", + "code": "\n function DoubleCounter() {\n const [count, setCount] = useState({ value: 0 });\n const [doubleCount, setDoubleCount] = useState({ value: 0 });\n\n useEffect(() => {\n setDoubleCount({ value: count.value * 2 });\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 16, + "name": "Optional chaining and nullish coalescing", + "code": "\n function DoubleCounter({ count }) {\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => {\n setDoubleCount((count?.value ?? 1) * 2);\n }, [count?.value]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 17, + "name": "Member access in effect body but not in deps", + "code": "\n function DoubleCounter(props) {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => setDoubleCount(props.count * 2), [props]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 18, + "name": "Doubly nested scopes in effect body", + "code": "\n const DoubleCounter = () => {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => {\n if (count > 10) {\n if (count > 100) {\n setDoubleCount(count * 4);\n } else {\n setDoubleCount(count * 2);\n }\n } else {\n setDoubleCount(count);\n }\n }, [count]);\n }\n ", + "todo": false, + "errors": 3 + }, + { + "idx": 19, + "name": "Destructured array skips element in variable declaration", + "code": "\n function SecondPost({ posts }) {\n const [secondPost, setSecondPost] = useState();\n\n useEffect(() => {\n const [, second] = posts;\n setSecondPost(second);\n }, [posts]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 20, + "name": "Value-less useState", + "code": "\n import { useState } from 'react';\n\n function AttemptCounter() {\n const [, setAttempts] = useState(0);\n const [count, setCount] = useState(0);\n\n useEffect(() => {\n setAttempts((prev) => {\n return prev + count;\n });\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 21, + "name": "Setter-less useState", + "code": "\n function AttemptCounter() {\n const [attempts, setAttempts] = useState(0);\n const [count] = useState(0);\n\n useEffect(() => {\n setAttempts(count);\n }, [count]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 22, + "name": "Custom hook with state", + "code": "\n function useCustomHook() {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n useEffect(() => {\n setDoubleCount(count * 2);\n }, [count]);\n\n return state;\n }\n\n function Component() {\n const customState = useCustomHook();\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 23, + "name": "FunctionDeclaration custom hook with props", + "code": "\n function useCustomHook(prop) {\n const [state, setState] = useState(0);\n\n useEffect(() => {\n setState(prop);\n }, [prop]);\n\n return state;\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 24, + "name": "VariableDeclarator custom hook with object props", + "code": "\n const useCustomHook = ({ prop }) => {\n const [state, setState] = useState(0);\n\n useEffect(() => {\n setState(prop);\n }, [prop]);\n\n return state;\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 25, + "name": "Conditional useEffect", + "code": "\n function DoubleCounter() {\n const [count, setCount] = useState(0);\n const [doubleCount, setDoubleCount] = useState(0);\n\n if (count > 10) {\n useEffect(() => {\n setDoubleCount(count * 2);\n }, [count]);\n }\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 26, + "name": "Passing non-anonymous function to effect", + "code": "\n function Form() {\n const [firstName, setFirstName] = useState('');\n const [lastName, setLastName] = useState('');\n const [name, setName] = useState('');\n\n function setDerivedName() {\n setName(firstName + ' ' + lastName);\n }\n\n useEffect(setDerivedName, [firstName, lastName]);\n }\n ", + "todo": true, + "errors": 1 + }, + { + "idx": 27, + "name": "Destructured array skips element in arrow function params", + "code": "\n function FilteredPosts({ posts }) {\n const [filteredPosts, setFilteredPosts] = useState([]);\n\n useEffect(() => {\n // Resulting AST node looks like:\n // {\n // \"type\": \"ArrayPattern\",\n // \"elements\": [\n // null, <-- Must handle this!\n // {\n // \"type\": \"Identifier\",\n // \"name\": \"second\"\n // }\n // ]\n // }\n setFilteredPosts(\n posts.filter(([, value]) => value !== \"\")\n );\n }, [posts]);\n }\n ", + "todo": false, + "errors": 1 + }, + { + "idx": 28, + "name": "Set derived state via identical intermediate setter", + "code": "\n const Component = () => {\n const [data, setData] = useState();\n // No idea why someone would do this. But good to know we catch it.\n // Passes when written as:\n // const onFetchedWrapper = (v) => onFetched(v);\n const onFetchedWrapper = onFetched;\n\n useEffect(() => {\n onFetchedWrapper(data);\n }, [onFetchedWrapper, data]);\n }\n ", + "todo": true, + "errors": 1 + } + ] +} diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-no-adjust-state-on-prop-change.test.ts b/packages/react-doctor/tests/regressions/state-rules/effect-no-adjust-state-on-prop-change.test.ts new file mode 100644 index 000000000..2d46dbce0 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-no-adjust-state-on-prop-change.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { collectRuleHits, createScopedTempRoot, setupReactProject } from "./_helpers.js"; + +const tempRoot = createScopedTempRoot("effect-no-adjust-state-on-prop-change"); + +const RULE_ID = "no-adjust-state-on-prop-change"; + +interface PortedCase { + caseId: string; + name: string; + code: string; + expectedErrors: number; +} + +// 1:1 port of upstream +// `eslint-plugin-react-you-might-not-need-an-effect/test/rules/no-adjust-state-on-prop-change.test.js`. +// Cases are kept in upstream order. Comments mirror upstream `name`. + +const valid: PortedCase[] = [ + { + caseId: "adjusting-state-during-render", + name: "Adjusting state directly during render", + code: `import { useState } from "react"; +export function List({ items }) { + const [isReverse, setIsReverse] = useState(false); + const [selection, setSelection] = useState(null); + + const [prevItems, setPrevItems] = useState(items); + if (items !== prevItems) { + setPrevItems(items); + setSelection(null); + } + return null; +} +`, + expectedErrors: 0, + }, + { + caseId: "set-state-literal-on-internal-state", + name: "Set state to literal when internal state changes", + code: `import { useEffect, useState } from "react"; +export function Counter() { + const [count, setCount] = useState(0); + const [otherState, setOtherState] = useState(); + + useEffect(() => { + setOtherState("Hello World"); + }, [count]); + return null; +} +`, + expectedErrors: 0, + }, + { + caseId: "set-state-derived-from-props", + name: "Set state to a value derived from props", + code: `import { useEffect, useState } from "react"; +export function Counter({ count }) { + const [doubleCount, setDoubleCount] = useState(0); + + useEffect(() => { + setDoubleCount(count * 2); + }, [count]); + return null; +} +`, + expectedErrors: 0, + }, +]; + +const invalid: PortedCase[] = [ + { + caseId: "literal-on-prop-change", + name: "Set state to literal when prop changes", + code: `import { useEffect, useState } from "react"; +export function List({ items }) { + const [selection, setSelection] = useState(); + + useEffect(() => { + setSelection(null); + }, [items]); + return null; +} +`, + expectedErrors: 1, + }, + { + caseId: "internal-state-on-prop-change", + name: "Set state to internal state when prop changes", + code: `import { useEffect, useState } from "react"; +export function List({ items }) { + const [selection, setSelection] = useState(); + const [internalData, setInternalData] = useState(); + + useEffect(() => { + setSelection(internalData); + }, [items, internalData]); + return null; +} +`, + expectedErrors: 1, + }, + { + caseId: "external-state-on-prop-change", + name: "Set state to external state when prop changes", + code: `import { useEffect, useState } from "react"; +declare const useDataSource: () => { data: unknown }; +export function List({ items }) { + const [selection, setSelection] = useState(); + const { data: externalData } = useDataSource(); + + useEffect(() => { + setSelection(externalData); + }, [items]); + return null; +} +`, + expectedErrors: 1, + }, + { + caseId: "conditional-literal-on-prop-change", + name: "Conditionally set state to literal when prop changes", + code: `import { useEffect, useState } from "react"; +export function Form({ result }) { + const [error, setError] = useState(); + + useEffect(() => { + if (result.data) { + setError(null); + } + }, [result]); + return null; +} +`, + expectedErrors: 1, + }, +]; + +describe(`${RULE_ID} (port of eslint-plugin-react-you-might-not-need-an-effect)`, () => { + for (const portedCase of valid) { + it(`valid: ${portedCase.name}`, async () => { + const projectDir = setupReactProject(tempRoot, portedCase.caseId, { + files: { "src/Component.tsx": portedCase.code }, + }); + const hits = await collectRuleHits(projectDir, RULE_ID); + expect(hits).toHaveLength(portedCase.expectedErrors); + }); + } + + for (const portedCase of invalid) { + it(`invalid: ${portedCase.name}`, async () => { + const projectDir = setupReactProject(tempRoot, portedCase.caseId, { + files: { "src/Component.tsx": portedCase.code }, + }); + const hits = await collectRuleHits(projectDir, RULE_ID); + expect(hits).toHaveLength(portedCase.expectedErrors); + }); + } +}); diff --git a/packages/react-doctor/tests/regressions/state-rules/effect-no-derived-state.test.ts b/packages/react-doctor/tests/regressions/state-rules/effect-no-derived-state.test.ts new file mode 100644 index 000000000..a1c87e1a7 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/effect-no-derived-state.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { collectRuleHits, createScopedTempRoot, setupReactProject } from "./_helpers.js"; + +const tempRoot = createScopedTempRoot("effect-no-derived-state"); + +describe("no-derived-state (port of eslint-plugin-react-you-might-not-need-an-effect)", () => { + it("does NOT flag computing in render from internal state", async () => { + const projectDir = setupReactProject(tempRoot, "valid-compute-in-render-internal", { + files: { + "src/Form.tsx": `import { useState } from "react"; + +export const Form = () => { + const [firstName] = useState("Taylor"); + const [lastName] = useState("Swift"); + const fullName = firstName + " " + lastName; + return {fullName}; +}; +`, + }, + }); + + expect(await collectRuleHits(projectDir, "no-derived-state")).toHaveLength(0); + }); + + it("does NOT flag computing in render from props", async () => { + const projectDir = setupReactProject(tempRoot, "valid-compute-in-render-props", { + files: { + "src/Form.tsx": `export const Form = ({ firstName, lastName }: { firstName: string; lastName: string }) => { + const fullName = firstName + " " + lastName; + return {fullName}; +}; +`, + }, + }); + + expect(await collectRuleHits(projectDir, "no-derived-state")).toHaveLength(0); + }); + + it("does NOT flag setting to literal on external state change", async () => { + const projectDir = setupReactProject(tempRoot, "valid-literal-on-external-state", { + files: { + "src/Feed.tsx": `import { useEffect, useState } from "react"; +declare const useQuery: (path: string) => { data: unknown[] }; + +export const Feed = () => { + const { data: posts } = useQuery("/posts"); + const [scrollPosition, setScrollPosition] = useState(0); + useEffect(() => { + setScrollPosition(0); + }, [posts]); + return
{scrollPosition}
; +}; +`, + }, + }); + + expect(await collectRuleHits(projectDir, "no-derived-state")).toHaveLength(0); + }); + + it("does NOT flag fetching external state on mount", async () => { + const projectDir = setupReactProject(tempRoot, "valid-fetch-on-mount", { + files: { + "src/Todos.tsx": `import { useEffect, useState } from "react"; + +export const Todos = () => { + const [todos, setTodos] = useState([]); + useEffect(() => { + fetch("/todos").then((response) => response.json()).then(setTodos); + }, []); + return
{todos.length}
; +}; +`, + }, + }); + + expect(await collectRuleHits(projectDir, "no-derived-state")).toHaveLength(0); + }); + + it("flags derived state from internal state", async () => { + const projectDir = setupReactProject(tempRoot, "invalid-internal-state", { + files: { + "src/Form.tsx": `import { useEffect, useState } from "react"; + +export const Form = () => { + const [firstName] = useState("Taylor"); + const [lastName] = useState("Swift"); + const [fullName, setFullName] = useState(""); + useEffect(() => setFullName(firstName + " " + lastName), [firstName, lastName]); + return {fullName}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-derived-state"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("fullName"); + expect(hits[0].message).toContain("derived state"); + }); + + it("flags derived state from props", async () => { + const projectDir = setupReactProject(tempRoot, "invalid-from-props", { + files: { + "src/Form.tsx": `import { useEffect, useState } from "react"; + +export const Form = ({ firstName, lastName }: { firstName: string; lastName: string }) => { + const [fullName, setFullName] = useState(""); + useEffect(() => { + setFullName(firstName + " " + lastName); + }, [firstName, lastName]); + return {fullName}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-derived-state"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("fullName"); + }); + + it("flags single-setter derived state from external", async () => { + const projectDir = setupReactProject(tempRoot, "invalid-single-setter-external", { + files: { + "src/Feed.tsx": `import { useEffect, useState } from "react"; +declare const fetchQuery: (path: string) => { data: unknown[] }; + +export const Feed = () => { + const { data: posts } = fetchQuery("/posts"); + const [selectedPost, setSelectedPost] = useState(); + useEffect(() => { + setSelectedPost(posts[0]); + }, [posts, setSelectedPost]); + return
{String(selectedPost)}
; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-derived-state"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("selectedPost"); + expect(hits[0].message).toContain("only set here"); + }); + + it("flags derived state via intermediate variable", async () => { + const projectDir = setupReactProject(tempRoot, "invalid-intermediate-variable", { + files: { + "src/Form.tsx": `import { useEffect, useState } from "react"; + +export const Form = ({ title }: { title: string }) => { + const [name] = useState("Dwayne"); + const [fullName, setFullName] = useState(""); + useEffect(() => { + const newFullName = title + " " + name; + setFullName(newFullName); + }, [title, name]); + return {fullName}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-derived-state"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("fullName"); + }); + + it("does NOT flag subscription effect (with cleanup)", async () => { + const projectDir = setupReactProject(tempRoot, "valid-subscription-effect", { + files: { + "src/Status.tsx": `import { useEffect, useState } from "react"; +declare const subscribeToStatus: (topic: string, cb: (s: string) => void) => () => void; + +export const Status = ({ topic }: { topic: string }) => { + const [status, setStatus] = useState(); + useEffect(() => { + const unsubscribe = subscribeToStatus(topic, (newStatus) => { + setStatus(newStatus); + }); + return () => unsubscribe(); + }, [topic]); + return
{status}
; +}; +`, + }, + }); + + expect(await collectRuleHits(projectDir, "no-derived-state")).toHaveLength(0); + }); +}); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-adjust-state-on-prop-change.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-adjust-state-on-prop-change.test.ts new file mode 100644 index 000000000..b1861d59b --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-adjust-state-on-prop-change.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-adjust-state-on-prop-change"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-chain-state-updates.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-chain-state-updates.test.ts new file mode 100644 index 000000000..d5524817c --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-chain-state-updates.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-chain-state-updates"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-derived-state.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-derived-state.test.ts new file mode 100644 index 000000000..a7b7ed836 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-derived-state.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-derived-state"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-event-handler.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-event-handler.test.ts new file mode 100644 index 000000000..8d39016d7 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-event-handler.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-event-handler"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-initialize-state.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-initialize-state.test.ts new file mode 100644 index 000000000..cba859743 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-initialize-state.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-initialize-state"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-pass-data-to-parent.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-pass-data-to-parent.test.ts new file mode 100644 index 000000000..bb4a852cc --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-pass-data-to-parent.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-pass-data-to-parent"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-pass-live-state-to-parent.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-pass-live-state-to-parent.test.ts new file mode 100644 index 000000000..7600ab426 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-pass-live-state-to-parent.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-pass-live-state-to-parent"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-no-reset-all-state-on-prop-change.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-no-reset-all-state-on-prop-change.test.ts new file mode 100644 index 000000000..4669a5706 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-no-reset-all-state-on-prop-change.test.ts @@ -0,0 +1,3 @@ +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("no-reset-all-state-on-prop-change"); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-real-world.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-real-world.test.ts new file mode 100644 index 000000000..166de9c42 --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-real-world.test.ts @@ -0,0 +1,7 @@ +// 1:1 port of upstream `test/real-world.test.js` — drives the full +// recommended config (all 8 rules) against real-world React snippets +// and asserts none of them fire. Mirrors upstream's +// `describe("recommended rules on real-world code", ...)` block. +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("real-world", { assertNoneOfPortedRules: true }); diff --git a/packages/react-doctor/tests/regressions/state-rules/parity-syntax.test.ts b/packages/react-doctor/tests/regressions/state-rules/parity-syntax.test.ts new file mode 100644 index 000000000..e987d05cb --- /dev/null +++ b/packages/react-doctor/tests/regressions/state-rules/parity-syntax.test.ts @@ -0,0 +1,9 @@ +// 1:1 port of upstream `test/syntax.test.js` — drives `no-derived-state` +// with 31 syntactic variations (different `useEffect` import shapes, +// arrow vs function components, single vs multi-statement effect bodies, +// `useCallback` wrappers, etc.). These exist upstream as a smoke test +// for the scope-based ast/react helpers; we replay them through our +// eslint-scope-backed analyzer for the same coverage. +import { runUpstreamParity } from "./_effect-parity-runner.js"; + +runUpstreamParity("syntax", { ruleId: "no-derived-state" }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 168b7a1b1..f19eaf336 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -58,9 +58,6 @@ importers: eslint-plugin-react-hooks: specifier: ^7.1.1 version: 7.1.1(eslint@9.39.2(jiti@2.6.1)) - eslint-plugin-react-you-might-not-need-an-effect: - specifier: ^0.10.1 - version: 0.10.1(eslint@9.39.2(jiti@2.6.1)) packages/eslint-plugin-react-doctor: dependencies: @@ -74,9 +71,21 @@ importers: packages/oxlint-plugin-react-doctor: dependencies: + '@types/eslint-scope': + specifier: ^9.1.0 + version: 9.1.0 + '@types/eslint-visitor-keys': + specifier: ^3.3.2 + version: 3.3.2 '@typescript-eslint/types': specifier: ^8.59.3 version: 8.59.3 + eslint-scope: + specifier: ^9.1.2 + version: 9.1.2 + eslint-visitor-keys: + specifier: ^5.0.1 + version: 5.0.1 devDependencies: '@react-doctor/types': specifier: workspace:* @@ -137,9 +146,6 @@ importers: eslint-plugin-react-hooks: specifier: ^7.1.1 version: 7.1.1(eslint@9.39.2(jiti@2.6.1)) - eslint-plugin-react-you-might-not-need-an-effect: - specifier: ^0.10.1 - version: 0.10.1(eslint@9.39.2(jiti@2.6.1)) packages/types: devDependencies: @@ -1348,6 +1354,17 @@ packages: '@types/deep-eql@4.0.2': resolution: {integrity: sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw==} + '@types/eslint-scope@9.1.0': + resolution: {integrity: sha512-ZdRd/C3bTObl4fZ6d7iXN2AVv20waMU2tPy1W24e6ECLbB8T9fBxtONKZUpIpYHShMVk6cDNP9QMVw7Qdqsd/w==} + deprecated: This is a stub types definition. eslint-scope provides its own type definitions, so you do not need this installed. + + '@types/eslint-visitor-keys@3.3.2': + resolution: {integrity: sha512-Jv7Ga0rieI+HtsG18BwW7FJZbxtm5XZxPRbQN9mSyek6Dt29YEkeWGxnUvwwzQ76j8BrS5nhfLF2tch+9vesuQ==} + deprecated: This is a stub types definition. eslint-visitor-keys provides its own type definitions, so you do not need this installed. + + '@types/esrecurse@4.3.1': + resolution: {integrity: sha512-xJBAbDifo5hpffDBuHl0Y8ywswbiAp/Wi7Y/GtAgSlZyIABppyurxVueOPE8LUQOxdlgi6Zqce7uoEpqNTeiUw==} + '@types/estree@1.0.8': resolution: {integrity: sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w==} @@ -1740,16 +1757,14 @@ packages: peerDependencies: eslint: ^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0 || ^10.0.0 - eslint-plugin-react-you-might-not-need-an-effect@0.10.1: - resolution: {integrity: sha512-IK0s/+ShN0bkur5moKCu/lfx2D/9uIeozje8Wv2/XnYdmswa17pDg02aUuytEPb8Gf0eueiQFf/QsvOHHcvujg==} - engines: {node: '>=14.0.0'} - peerDependencies: - eslint: '>=8.40.0' - eslint-scope@8.4.0: resolution: {integrity: sha512-sNXOfKCn74rt8RICKMvJS7XKV/Xk9kA7DyJr8mJik3S7Cwgy3qlkkmyS2uQB3jiJg6VNdZd/pDBJu0nvG2NlTg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + eslint-scope@9.1.2: + resolution: {integrity: sha512-xS90H51cKw0jltxmvmHy2Iai1LIqrfbw57b79w/J7MfvDfkIkFZ+kj6zC3BjtUwh150HsSSdxXZcsuv72miDFQ==} + engines: {node: ^20.19.0 || ^22.13.0 || >=24} + eslint-visitor-keys@3.4.3: resolution: {integrity: sha512-wpc+LXeiyiisxPlEkUzU6svyS1frIO3Mgxj1fdy7Pm8Ygzguax2N3Fa/D/ag1WqbOprdI+uY6wMUl8/a2G+iag==} engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} @@ -1758,6 +1773,10 @@ packages: resolution: {integrity: sha512-Uhdk5sfqcee/9H/rCOJikYz67o0a2Tw2hGRPOG2Y1R2dg7brRe1uG0yaNQDHu+TO/uQPF/5eCapvYSmHUjt7JQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + eslint-visitor-keys@5.0.1: + resolution: {integrity: sha512-tD40eHxA35h0PEIZNeIjkHoDR4YjjJp34biM0mDvplBe//mB+IHCqHDGV7pxF+7MklTvighcCPPZC7ynWyjdTA==} + engines: {node: ^20.19.0 || ^22.13.0 || >=24} + eslint@9.39.2: resolution: {integrity: sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} @@ -1877,10 +1896,6 @@ packages: resolution: {integrity: sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ==} engines: {node: '>=18'} - globals@16.5.0: - resolution: {integrity: sha512-c/c15i26VrJ4IRt5Z89DnIzCGDn9EcebibhAOjw5ibqEHsE1wLUgkPn9RDmNcUKyU87GeaL633nyJ+pplFR2ZQ==} - engines: {node: '>=18'} - globby@11.1.0: resolution: {integrity: sha512-jhIXaOzy1sb8IyocaruWSn1TjmnBVs8Ayhcy83rmxNJ8q2uWKCAj3CnJY+KpGSXCueAPc0i05kVvVKtP1t9S3g==} engines: {node: '>=10'} @@ -3530,6 +3545,16 @@ snapshots: '@types/deep-eql@4.0.2': {} + '@types/eslint-scope@9.1.0': + dependencies: + eslint-scope: 9.1.2 + + '@types/eslint-visitor-keys@3.3.2': + dependencies: + eslint-visitor-keys: 5.0.1 + + '@types/esrecurse@4.3.1': {} + '@types/estree@1.0.8': {} '@types/json-schema@7.0.15': {} @@ -3828,13 +3853,15 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-plugin-react-you-might-not-need-an-effect@0.10.1(eslint@9.39.2(jiti@2.6.1)): + eslint-scope@8.4.0: dependencies: - eslint: 9.39.2(jiti@2.6.1) - globals: 16.5.0 + esrecurse: 4.3.0 + estraverse: 5.3.0 - eslint-scope@8.4.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 @@ -3842,6 +3869,8 @@ snapshots: eslint-visitor-keys@4.2.1: {} + eslint-visitor-keys@5.0.1: {} + eslint@9.39.2(jiti@2.6.1): dependencies: '@eslint-community/eslint-utils': 4.9.1(eslint@9.39.2(jiti@2.6.1)) @@ -3981,8 +4010,6 @@ snapshots: globals@14.0.0: {} - globals@16.5.0: {} - globby@11.1.0: dependencies: array-union: 2.1.0