Skip to content

Add eslint-plugin-react-you-might-not-need-an-effect#8000

Merged
mattcosta7 merged 1 commit into
mainfrom
add-no-unnecessary-effect-eslint-plugin
Jun 16, 2026
Merged

Add eslint-plugin-react-you-might-not-need-an-effect#8000
mattcosta7 merged 1 commit into
mainfrom
add-no-unnecessary-effect-eslint-plugin

Conversation

@mattcosta7

Copy link
Copy Markdown
Contributor

Closes #

What

Adds eslint-plugin-react-you-might-not-need-an-effect and wires it into our flat ESLint config via its recommended preset. The plugin flags useEffect usage that the React docs describe under "You Might Not Need an Effect" — storing derived state, running event-handler logic, chaining state updates, resetting/adjusting state on prop changes, passing data up to parents, subscribing to external stores, and initializing state in an effect.

Because the codebase already contains effects that trip these rules, the pre-existing violations are suppressed inline with // eslint-disable-next-line comments (107 sites across 37 files) rather than disabling the rules globally. New code is linted immediately; each legacy case is individually marked so it can be revisited and removed over time.

Why

  • The "You Might Not Need an Effect" anti-patterns are a recurring source of bugs — extra renders, stale state, and render loops — and they make components harder to reason about. Catching them at lint time keeps new code on the rails.
  • Suppressing with inline disables (instead of a config-level ignore list) keeps every remaining case visible at its call site and discoverable via grep/code search, so the backlog can be burned down incrementally. A central ignore list would hide the debt and silently swallow new violations in those files.

How

  • eslint.config.mjs: import the plugin and add reactYouMightNotNeedAnEffect.configs.recommended (enables all 9 rules as warnings across JS/TS files).
  • Suppression uses per-line // eslint-disable-next-line comments — the repo's eslint-comments/no-use rule bans whole-file /* eslint-disable */ blocks, and per-line directives are the convention already used throughout the repo.
  • Where a line already had a directive (e.g. react-hooks/set-state-in-effect), the new rule id was merged into the existing comment rather than added on a separate line, to avoid "unused directive" errors (lint runs with --max-warnings=0).
  • No runtime/source logic changed — every code edit is an added or merged lint comment.

Changelog

New

  • Dev dependency eslint-plugin-react-you-might-not-need-an-effect and its recommended ESLint configuration.

Changed

  • Suppressed pre-existing unnecessary-effect violations inline across 37 files. No behavior changes.

Removed

  • N/A

Rollout strategy

  • None; this is dev-only lint tooling. There are no changes to the published @primer/react package, so no changeset is required (the skip changeset label is applied).

Testing & Reviewing

  • npm run lint passes with --max-warnings=0 — zero remaining plugin violations and zero unused disable directives.
  • Since no source logic changed, the diff should be reviewable as purely additive lint comments. The interesting file is eslint.config.mjs; everything else is // eslint-disable-next-line annotations.
  • Follow-up: the inline disables mark a backlog of effects that are good candidates for refactoring away.

Merge checklist

  • Added/updated tests — N/A (tooling only)
  • Added/updated documentation — N/A
  • Added/updated previews (Storybook) — N/A
  • Changes are SSR compatible (no runtime changes)
  • Tested in Chrome / Firefox / Safari / Edge — N/A (no runtime changes)

Enable the plugin's recommended preset to catch unnecessary React
effects. Existing violations are suppressed inline with
eslint-disable-next-line comments so they can be refactored over time.
@mattcosta7 mattcosta7 added the skip changeset This change does not need a changelog label Jun 16, 2026 — with GitHub Codespaces
@changeset-bot

changeset-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8147fc3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jun 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@mattcosta7 mattcosta7 self-assigned this Jun 16, 2026
@mattcosta7 mattcosta7 added integration-tests: skipped manually Changes in this PR do not require an integration test and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jun 16, 2026
@mattcosta7 mattcosta7 marked this pull request as ready for review June 16, 2026 13:05
@mattcosta7 mattcosta7 requested a review from a team as a code owner June 16, 2026 13:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request introduces the eslint-plugin-react-you-might-not-need-an-effect ESLint plugin and enables its recommended preset in the repo’s flat ESLint config, then suppresses all pre-existing violations inline so new code is linted immediately without introducing warnings.

Changes:

  • Add eslint-plugin-react-you-might-not-need-an-effect@1.0.1 to dev dependencies and wire its recommended config into eslint.config.mjs.
  • Add/merge // eslint-disable-next-line react-you-might-not-need-an-effect/... directives across existing useEffect sites to suppress legacy violations.
  • Update package-lock.json to reflect the new dependency (and additional workspace/version changes that appear unrelated to the described scope).
Show a summary per file
File Description
eslint.config.mjs Imports the plugin and enables its configs.recommended preset in the flat config array.
package.json Adds the new ESLint plugin to root devDependencies.
package-lock.json Adds the new plugin to the lockfile; also includes workspace/version bumps that don’t match the PR’s stated scope.
packages/react/src/utils/StressTest.tsx Adds inline disables for plugin findings in an existing effect.
packages/react/src/utils/descendant-registry.tsx Adds inline disable for no-pass-data-to-parent on an existing state update.
packages/react/src/TreeView/TreeView.tsx Merges plugin rule disables into existing eslint-disable-next-line directives and adds a few new ones.
packages/react/src/TooltipV2/Tooltip.tsx Adds inline disable for no-event-handler in an existing effect.
packages/react/src/ToggleSwitch/ToggleSwitch.tsx Merges/introduces inline disables around existing effect-driven state updates.
packages/react/src/Textarea/Textarea.tsx Adds inline disables for plugin findings in existing character counter effects.
packages/react/src/Spinner/Spinner.tsx Adds inline disable for an existing delay effect.
packages/react/src/SelectPanel/SelectPanel.tsx Adds multiple inline disables across existing effects (loading, focus, announcements, sorting).
packages/react/src/SelectPanel/SelectPanel.features.stories.tsx Adds inline disables in story effects to suppress legacy findings.
packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx Adds inline disables in example story effects to suppress legacy findings.
packages/react/src/SelectPanel/SelectPanel.dev.stories.tsx Adds inline disables in dev story effect to suppress legacy findings.
packages/react/src/Portal/Portal.features.stories.tsx Adds inline disables for no-initialize-state in existing mount effects.
packages/react/src/Overlay/Overlay.features.stories.tsx Adds inline disable for an existing focus-on-editing effect.
packages/react/src/LabelGroup/LabelGroup.tsx Adds inline disables in existing truncation/overflow-related effects.
packages/react/src/internal/components/ValidationAnimationContainer.tsx Merges plugin disables into an existing state-in-effect suppression.
packages/react/src/hooks/useMnemonics.ts Adds inline disable in an existing effect that mutates DOM attributes.
packages/react/src/hooks/useMenuInitialFocus.ts Adds inline disable for an effect that moves focus on open.
packages/react/src/hooks/useMedia.ts Merges plugin disable with existing set-state-in-effect suppression.
packages/react/src/hooks/useFocusZone.ts Adds inline disable within existing effect guard conditions.
packages/react/src/hooks/useDialog.ts Adds inline disables around existing “open” effects (listeners, focus).
packages/react/src/hooks/useDetails.tsx Adds inline disable for an existing click-outside effect.
packages/react/src/hooks/useControllableState.ts Adds inline disables in an effect that emits controlled/uncontrolled warnings.
packages/react/src/FormControl/FormControl.features.stories.tsx Adds inline disables around story-only validation state effects.
packages/react/src/FilteredActionList/useAnnouncements.tsx Adds inline disable for an existing announcement condition inside an effect.
packages/react/src/FilteredActionList/FilteredActionList.tsx Adds inline disables for effects that pass refs upward / manage roving tabindex state.
packages/react/src/experimental/SelectPanel2/SelectPanel.tsx Adds inline disable for an existing initial focus effect.
packages/react/src/experimental/SelectPanel2/SelectPanel.examples.stories.tsx Merges plugin disable into an existing state-in-effect suppression.
packages/react/src/Dialog/Dialog.tsx Merges plugin disable into existing state-in-effect suppression.
packages/react/src/Dialog/Dialog.features.stories.tsx Adds inline disable for an existing step-focus effect.
packages/react/src/DataTable/storybook/data.ts Adds inline disables around an existing query effect’s state resets.
packages/react/src/Breadcrumbs/Breadcrumbs.tsx Adds inline disables around an existing overflow-calculation effect.
packages/react/src/AvatarStack/AvatarStack.tsx Adds inline disable for an existing “initialize on mount” effect call.
packages/react/src/Autocomplete/AutocompleteMenu.tsx Adds inline disables for effects that sync derived state / notify parent / compute suggestion text.
packages/react/src/Autocomplete/AutocompleteInput.tsx Adds inline disable within existing logic that highlights remaining text.
packages/react/src/Autocomplete/Autocomplete.features.stories.tsx Adds inline disable for a story mount effect state update.
packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx Adds inline disable for an effect that clears overlay ref on close.
packages/react/src/ActionList/Description.tsx Adds inline disables for effect-driven DOM text extraction to compute tooltip/title.

Copilot's findings

  • Files reviewed: 39/40 changed files
  • Comments generated: 0

@mattcosta7 mattcosta7 added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit 5b07b9a Jun 16, 2026
70 of 71 checks passed
@mattcosta7 mattcosta7 deleted the add-no-unnecessary-effect-eslint-plugin branch June 16, 2026 15:38
Copilot AI added a commit that referenced this pull request Jun 16, 2026
Resolve conflicts from Add eslint-plugin-react-you-might-not-need-an-effect (#8000):
- Keep useSyncExternalStore implementation in useMedia.ts (our refactor)
- Keep render-time derivation in ValidationAnimationContainer.tsx (our refactor)
- Keep render-time derivation in SelectPanel.tsx intermediateSelected (our refactor)
- Keep render-time derivation in ToggleSwitch.tsx; add no-event-handler suppression
- Merge descriptive comments with new react-you-might-not-need-an-effect suppressions
  in Dialog.tsx, AutocompleteMenu.tsx, SelectPanel.tsx, and TreeView.tsx

Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: skipped manually Changes in this PR do not require an integration test skip changeset This change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants