From a83d8bf3ac9ba6e6861c75bb3670b9b8d630809a Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:42:17 -0700 Subject: [PATCH] chore(lint): a new React effect states its reason instead of raising the budget The budget rule told authors to try a ref callback or a store, which rewarded hiding lifecycle work instead of judging an effect by its purpose. An effect with `// oxlint-disable-next-line t3code/react-effect-budget -- ` above it is now left out of the count, so the reason sits on the effect where reviewers see it. The message asks whether the work is synchronization that belongs in render or a handler, and no longer suggests refs or stores. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- .../rules/react-effect-budget.test.ts | 10 ++++++++++ .../rules/react-effect-budget.ts | 19 ++++++++++++++++--- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e0af56a8ade66..01666d91ec0b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -194,7 +194,7 @@ Architecture and its constraints: `docs/internals/overview.md`. Glossary: `docs/ options, and pass combinators as additional function-builder arguments. - Inferred types over annotations. `any` is the enemy. - Default to no comment. Write one only for a constraint, an external quirk, or an invariant the code can't show, in one line where you can and four at most. Don't restate names or signatures, narrate steps, record history, or explain the fix you just made; the PR carries that. `t3code/no-low-value-comments` enforces the length cap and the mechanical cases. -- React effects are a last resort: derive in render, act in handlers, reset with `key`, or use a ref callback or store. The `t3code/react-effect-budget` lint rule holds each file to its entry in `scripts/effect-budget.json`; raise an entry only in the PR that needs the effect, and lower it when you remove one. +- React effects are for synchronizing with something outside React. Derive in render and act in handlers instead of syncing state, but don't hide real lifecycle work in refs or stores to dodge one. The `t3code/react-effect-budget` lint rule holds each file to its count in `scripts/effect-budget.json`; a new effect states its reason with `// oxlint-disable-next-line t3code/react-effect-budget -- `. - Our users drive agents all day and notice a dropped frame, a lying spinner, and a stale label. No continuously repainting animations; they peg the GPU on high-refresh displays. - If a rule here fights the task in front of you, say so loudly and get a human sign-off before breaking it. diff --git a/oxlint-plugin-t3code/rules/react-effect-budget.test.ts b/oxlint-plugin-t3code/rules/react-effect-budget.test.ts index b1cba759d188a..1841ba350bd34 100644 --- a/oxlint-plugin-t3code/rules/react-effect-budget.test.ts +++ b/oxlint-plugin-t3code/rules/react-effect-budget.test.ts @@ -46,5 +46,15 @@ const alias = useEffect; `, ); + unbudgeted.valid( + "does not count an effect that states its reason", + `export function New() { + // oxlint-disable-next-line t3code/react-effect-budget -- subscribes to the window's resize events + React.useEffect(() => {}); + return null; +} +`, + ); + lint("apps/web/src/Panel.test.tsx").valid("skips test files", component(3)); }); diff --git a/oxlint-plugin-t3code/rules/react-effect-budget.ts b/oxlint-plugin-t3code/rules/react-effect-budget.ts index 3974eb6e7b9ab..231c707457505 100644 --- a/oxlint-plugin-t3code/rules/react-effect-budget.ts +++ b/oxlint-plugin-t3code/rules/react-effect-budget.ts @@ -9,6 +9,12 @@ const EFFECT_HOOKS = new Set(["useEffect", "useLayoutEffect", "useInsertionEffec const EFFECT_HOOK_TEXT = /use(?:Layout|Insertion)?Effect/; const UNBUDGETED_FILE = /\.test\.[cm]?[jt]sx?$|[\\/](?:test|__tests__)[\\/]/; const budgets: Readonly> = budget; +const JUSTIFIED = /oxlint-disable-next-line[^\n]*t3code\/react-effect-budget\s+--\s*\S/; + +// An effect with a stated reason on the line above is a reviewed decision: it is not counted, and it +// is always reported so that the suppression comment above it is used. +const isJustified = (lines: ReadonlyArray, line: number): boolean => + JUSTIFIED.test(lines[line - 2] ?? ""); const isEffectHookCall = (node: ESTree.CallExpression): boolean => { const callee = unwrapExpression(node.callee); @@ -39,18 +45,25 @@ export default defineRule({ if (UNBUDGETED_FILE.test(context.filename) || !EFFECT_HOOK_TEXT.test(context.sourceCode.text)) { return {}; } + const lines = context.sourceCode.text.split("\n"); const calls: Array = []; + const justified: Array = []; return { CallExpression(node) { - if (isEffectHookCall(node)) calls.push(node); + if (!isEffectHookCall(node)) return; + (isJustified(lines, node.loc.start.line) ? justified : calls).push(node); }, "Program:exit"() { + for (const node of justified) { + context.report({ node, message: "A React effect with a stated reason." }); + } const allowed = budgetFor(context.filename); if (calls.length <= allowed) return; const message = `This file has ${calls.length} React ${calls.length === 1 ? "effect" : "effects"} against a budget of ${allowed}. ` + - "Prefer deriving in render, event handlers, a `key` reset, an element-owned ref callback, or a store. " + - `If an effect is genuinely right, raise this file's entry in ${BUDGET_FILE} in the same PR.`; + "First ask whether this is synchronization that belongs in render or in the handler that causes it. " + + "If an effect is the honest model, say why on the line above it: " + + "`// oxlint-disable-next-line t3code/react-effect-budget -- `."; for (const node of calls.slice(allowed)) context.report({ node, message }); }, };