Pattern creator: fix react-hooks/exhaustive-deps lint warnings#748
Merged
Conversation
Clear the eslint-plugin-react-hooks warnings CI surfaces in the pattern-creator workspace: - Add genuinely-missing dependencies read from a prop or store inside the hook: `postId` (editor useSelect, url-controller useEffect), `feature` (feature-toggle useSelect), `onSelect`/`onClose` (openverse grid useCallback). - Add stable dispatchers/setters to satisfy the rule without changing behavior: `createInfoNotice`, `enableComplementaryArea`, `setDebouncedSearchTerm`. - Give the four pattern-settings `useCallback`s a dependency array; they previously had none and were not memoizing at all. - Document and silence two intentional omissions with eslint-disable-next-line: the pattern-categories-control `useMemo` (avoids reordering the list on selection) and the submission-modal `useEffect` (including `meta` would loop, `editPost` is stable). Add regression tests for the two behavior-affecting fixes (url-controller `postId`, feature-toggle `feature`); each was verified to fail without the fix. Tests render through the React copy `@wordpress/element` bundles to avoid the repo's dual-React invalid-hook-call issue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ep fixes Cover the two remaining behavior-affecting dependency fixes: - editor: re-renders with a new `postId` now look up the new post. The editor short-circuits to `null` without `siteUrl`, so the stores are stubbed with minimal stand-ins and the heavy children are mocked — the `useSelect` mapping (the unit under test) still runs. - openverse grid: `onCommitSelected` now calls the latest `onSelect` / `onClose` props after they change, rather than the props captured on first render. Both tests were verified to fail with the dependency arrays reverted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR removes react-hooks/exhaustive-deps warnings across the pattern-creator workspace by correcting hook dependency arrays (and documenting intentional omissions), and adds regression tests for the dependency-related behavioral fixes using @wordpress/element rendering to avoid React instance conflicts.
Changes:
- Fix missing/incorrect hook dependencies in key components (URL controller, editor selectors/effects, sidebar behavior, Openverse grid callbacks).
- Add documented/silenced intentional exhaustive-deps omissions where including deps would cause undesirable behavior/loops.
- Introduce new component-render regression tests (via
@wordpress/element+ matchingact) for the behavioral dependency fixes.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| public_html/wp-content/plugins/pattern-creator/src/components/url-controller/test/index.js | Adds regression coverage for URL updates, including postId-only changes. |
| public_html/wp-content/plugins/pattern-creator/src/components/url-controller/index.js | Fixes useEffect deps to include postId so URL updates don’t go stale. |
| public_html/wp-content/plugins/pattern-creator/src/components/submission-modal/index.js | Documents/silences intentional deps omission to avoid update loops while syncing editor state. |
| public_html/wp-content/plugins/pattern-creator/src/components/sidebar/pattern-settings/index.js | Adds missing dependency arrays to useCallbacks so they actually memoize and satisfy lint. |
| public_html/wp-content/plugins/pattern-creator/src/components/sidebar/index.js | Adds stable dispatcher to effect deps to satisfy exhaustive-deps. |
| public_html/wp-content/plugins/pattern-creator/src/components/pattern-categories-control/index.js | Documents/silences intentional deps omission in useMemo to keep ordering stable. |
| public_html/wp-content/plugins/pattern-creator/src/components/openverse/test/grid.js | Adds regression coverage ensuring commit uses latest onSelect/onClose props. |
| public_html/wp-content/plugins/pattern-creator/src/components/openverse/grid.js | Fixes effect/callback deps so debounced search and commit handler don’t capture stale values. |
| public_html/wp-content/plugins/pattern-creator/src/components/header/feature-toggle/test/index.js | Adds regression coverage that useSelect tracks changes to the feature prop. |
| public_html/wp-content/plugins/pattern-creator/src/components/header/feature-toggle/index.js | Fixes useSelect deps so active state isn’t captured from initial feature. |
| public_html/wp-content/plugins/pattern-creator/src/components/editor/test/index.js | Adds regression coverage that selector reads the current postId when it changes. |
| public_html/wp-content/plugins/pattern-creator/src/components/editor/index.js | Fixes useSelect and effect deps to avoid stale postId and satisfy exhaustive-deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replacing the built-in with a bare jest.fn() left it overwritten after the suite ran. Use jest.spyOn with mockImplementation and restore it in afterEach so the mock can't leak into other tests in the same worker. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`jest.mock()` factories are hoisted, and babel-plugin-jest-hoist only permits out-of-scope references that are clearly mock-only (prefixed `mock`). Rename the `makeStore` helper to `mockMakeStore` so the factory references stay within that guard and don't risk a hoist-time failure on stricter plugin versions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add `--max-warnings=0` to every workspace's `lint:js` script so any eslint warning (e.g. react-hooks/exhaustive-deps) fails the lint step locally and in CI, preventing regressions of the warnings just cleared from the pattern-creator. Also clear the one pre-existing warning this surfaces: the unlist-button notice effect was missing its `createNotice` / `removeNotice` deps. Both are stable `useDispatch` handles, so listing them is behavior-neutral. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| "format:js": "wp-scripts format src -- --config=../../../../.prettierrc.js", | ||
| "lint:css": "wp-scripts lint-style 'style.css' 'src/**/*.scss'", | ||
| "lint:js": "wp-scripts lint-js src", | ||
| "lint:js": "wp-scripts lint-js src --max-warnings=0", |
| "format:js": "wp-scripts format src -- --config=../../../../.prettierrc.js", | ||
| "lint:css": "wp-scripts lint-style 'src/**/*.scss'", | ||
| "lint:js": "wp-scripts lint-js src", | ||
| "lint:js": "wp-scripts lint-js src --max-warnings=0", |
| "build": "wp-scripts build --experimental-modules", | ||
| "start": "wp-scripts start --experimental-modules", | ||
| "lint:js": "wp-scripts lint-js src", | ||
| "lint:js": "wp-scripts lint-js src --max-warnings=0", |
Comment on lines
33
to
+36
| } else { | ||
| removeNotice( NOTICE_ID ); | ||
| } | ||
| }, [ status ] ); | ||
| }, [ status, createNotice, removeNotice ] ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Clears all
eslint-plugin-react-hooks(react-hooks/exhaustive-deps) warnings in the pattern-creator workspace. These surface as annotations on every PR's Static Analysis (Linting) job (e.g. run 26952016144). GitHub caps annotations at 10, but the job actually reported 13 warnings across 8 files — all are fixed here.Root cause
Each warning is a hook whose dependency array doesn't match what the hook body reads. They fall into three groups, fixed accordingly:
1. Genuinely missing dependencies (behavioral) — added the dep
url-controlleruseEffectpostIdpostIdchangededitoruseSelectpostIdpostIdchangedheader/feature-toggleuseSelectfeatureisActivewas stale when thefeatureprop changedopenverse/griduseCallbackonSelect,onClose2. Stable dispatchers / setters — added the dep (no behavior change)
createInfoNotice(editor),enableComplementaryArea(sidebar),setDebouncedSearchTerm(grid). These come fromuseDispatch/useDebounceand are stable, so listing them only satisfies the rule.3. The four
pattern-settingsuseCallbacks had no dependency array at all — they weren't memoizing. Gave each the correct array.Intentional omissions, now documented + silenced with
eslint-disable-next-line:pattern-categories-controluseMemo— omittingselectedTermsis deliberate (avoids reordering the list on every check/uncheck; mirrors Gutenberg's hierarchical-term-selector).submission-modaluseEffect— includingmetawould loop (the effect editsmeta), andeditPostis stable.Tests
Added regression tests for the four behavioral fixes — each verified to fail with the dependency array reverted:
url-controller(3 tests) — URL updates on status and onpostIdchangeheader/feature-toggle(3 tests) —isActivetracks thefeaturepropeditor(2 tests) — looks up the post for the currentpostIdopenverse/grid(2 tests) — commits to the latestonSelect/onCloseThe "stable dispatcher", memoization-only, and lint-suppression changes have no observable behavior change, so they aren't tested.
Note on test infrastructure
This workspace had no component-render tests before (only pure store tests), and
@wordpress/scriptsships Jest + jsdom but no React renderer. The repo also carries two React copies — root 19.x (forward-compat work) and the 18.x bundled in@wordpress/elementthat components actually render with — which trips "multiple copies of React" if you render via the root copy. The new tests render through@wordpress/element'screateRoot/createElementand pullactfrom that same copy, keeping a single React instance without any global Jest config change.🤖 Generated with Claude Code