chore: update eslint-plugin-react-hooks #9737
Conversation
|
|
View your CI Pipeline Execution ↗ for commit 35b8f89
☁️ Nx Cloud last updated this comment at |
WalkthroughESLint configs across examples and packages were updated to migrate React Hooks plugin usage to recommended-latest, adjust rules, and remove legacy/react-compiler settings. Several lint directives in source and tests were added/updated. Two functional tweaks landed: optionsRef sync moved to useEffect in HydrationBoundary, and a null-based init guard in useMutationState. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as App Component
participant HB as HydrationBoundary
participant QC as QueryClient
U->>HB: Render with props.options
note over HB: optionsRef.current not set during render
HB-->>U: Render output
rect rgb(236,248,255)
note right of HB: After commit (useEffect)
HB->>HB: optionsRef.current = props.options
HB->>QC: hydrate(newQueries, optionsRef.current)
QC-->>HB: Hydration complete
end
sequenceDiagram
autonumber
actor C as Component
participant HMS as useMutationState
participant MC as MutationCache
C->>HMS: Call hook
alt Initial render
HMS->>HMS: if (result.current === null) init from MC
end
HMS->>MC: subscribe(listener)
MC-->>HMS: notify on mutation changes
HMS-->>C: Return current result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Sizes for commit 35b8f89:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9737 +/- ##
=======================================
Coverage 45.51% 45.51%
=======================================
Files 196 196
Lines 8323 8324 +1
Branches 1894 1898 +4
=======================================
+ Hits 3788 3789 +1
Misses 4093 4093
Partials 442 442 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-query/src/__tests__/useQuery.test.tsx (1)
6687-6688: Remove or correct invalid ESLint disable comment
The rulereact-hooks/immutabilityisn’t defined in your ESLint config; update to a valid rule or remove the comment.
🧹 Nitpick comments (1)
packages/react-query/src/HydrationBoundary.tsx (1)
34-36: Add dependency array for efficiency.The useEffect runs after every render without a dependency array. Consider adding
[options]to only update the ref when options actually change:-React.useEffect(() => { - optionsRef.current = options -}) +React.useEffect(() => { + optionsRef.current = options +}, [options])This avoids unnecessary effect executions and makes the intent clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
examples/react/algolia/eslint.config.js(0 hunks)examples/react/basic-graphql-request/eslint.config.js(0 hunks)examples/react/basic/eslint.config.js(0 hunks)examples/react/shadow-dom/eslint.config.js(0 hunks)package.json(1 hunks)packages/react-query-devtools/eslint.config.js(1 hunks)packages/react-query-next-experimental/eslint.config.js(1 hunks)packages/react-query-next-experimental/src/HydrationStreamProvider.tsx(2 hunks)packages/react-query-persist-client/eslint.config.js(1 hunks)packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx(0 hunks)packages/react-query/eslint.config.js(1 hunks)packages/react-query/src/HydrationBoundary.tsx(2 hunks)packages/react-query/src/__tests__/useQuery.test.tsx(2 hunks)packages/react-query/src/__tests__/useSuspenseInfiniteQuery.test.tsx(1 hunks)packages/react-query/src/__tests__/useSuspenseQueries.test-d.tsx(1 hunks)packages/react-query/src/__tests__/useSuspenseQueries.test.tsx(2 hunks)packages/react-query/src/__tests__/useSuspenseQuery.test.tsx(1 hunks)packages/react-query/src/useMutationState.ts(1 hunks)
💤 Files with no reviewable changes (5)
- examples/react/algolia/eslint.config.js
- examples/react/shadow-dom/eslint.config.js
- packages/react-query-persist-client/src/tests/PersistQueryClientProvider.test.tsx
- examples/react/basic-graphql-request/eslint.config.js
- examples/react/basic/eslint.config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (17)
packages/react-query/src/__tests__/useSuspenseQueries.test.tsx (2)
261-262: LGTM! Appropriate test-specific suppression.The
react-hooks/puritysuppression is correct here. The test intentionally usesMath.random()to generate a unique identifier per component instance, verifying that the same component persists across multiple promise resolutions.
669-670: LGTM! Clever pattern for runtime validation testing.The suppression is appropriate. The
Math.random() >= 0condition is always true at runtime, but TypeScript cannot statically determine this, allowing the test to verify runtime error handling forskipTokenwithout triggering compile-time type errors.packages/react-query/src/__tests__/useSuspenseInfiniteQuery.test.tsx (1)
28-29: LGTM! Consistent with skipToken validation pattern.The
react-hooks/puritysuppression is appropriate. This follows the same pattern as other skipToken tests: usingMath.random() >= 0to create a condition that's always true at runtime but allows testing runtime validation without TypeScript interference.packages/react-query/src/__tests__/useSuspenseQuery.test.tsx (1)
878-879: LGTM! Consistent skipToken validation testing.The
react-hooks/puritysuppression is appropriate and consistent with the pattern used in other test files for validating runtimeskipTokenerror handling.packages/react-query/src/useMutationState.ts (1)
49-49: LGTM! More explicit initialization guard.The change from
!result.currenttoresult.current === nullmakes the initialization check more precise and intentional. Since the ref is initialized withnull, this explicit equality check avoids potential false positives from other falsy values and clearly communicates thatnullis the uninitialized sentinel.packages/react-query/src/HydrationBoundary.tsx (1)
94-95: LGTM! Eslint-disable is justified.The
react-hooks/refsdisable is appropriate here. The ref pattern intentionally avoids addingoptionsto theuseMemodependencies to prevent re-computation on options changes (as explained in the comments above).Note: If
clientorstatechanges at the same momentoptionschanges, theuseMemowill execute during render with the previous render'soptionsvalue (before theuseEffecton lines 34-36 updatesoptionsRef.current). Sincehydrateis idempotent (line 93 comment), this timing is likely safe, but keep in mind that the options used here may lag by one render in that edge case.package.json (1)
57-57: LGTM!The upgrade from RC (
^6.0.0-rc.2) to stable (^6.1.1) is appropriate and aligns with the corresponding ESLint config migrations across the repository.packages/react-query-next-experimental/src/HydrationStreamProvider.tsx (2)
133-133: LGTM!The lint directive correctly updated from
react-hooks/react-compilertoreact-hooks/immutability, reflecting the rule changes in eslint-plugin-react-hooks ^6.1.1. The suppression is appropriate for the intentional stream flush viastream.length = 0.
172-172: LGTM!The lint directive correctly updated from
react-hooks/react-compilertoreact-hooks/immutability. The suppression is appropriate for the intentional mutation ofwin[id]to set up the client-side hydration handler.packages/react-query-devtools/eslint.config.js (3)
4-4: LGTM!Import style correctly updated to default import, matching the new export structure of eslint-plugin-react-hooks ^6.1.1.
18-19: LGTM!The new rules
unsupported-syntaxandincompatible-libraryare appropriate additions from eslint-plugin-react-hooks ^6.1.1, replacing the previousreact-compilerrule.
9-10: Confirm necessity of@ts-expect-errorThe directive suppresses a type error when spreadingreactHooks.configs['recommended-latest']. Verify locally againsteslint-plugin-react-hooks@6.1.1typings and remove the workaround once the upstream type issue is resolved.packages/react-query-next-experimental/eslint.config.js (1)
4-4: LGTM!ESLint config changes are consistent with the eslint-plugin-react-hooks ^6.1.1 upgrade: default import,
recommended-latestconfig, and new rules (unsupported-syntax,incompatible-library). The samets-expect-errortype workaround applies here as in other package configs.Also applies to: 9-10, 18-19
packages/react-query/eslint.config.js (2)
4-4: LGTM!ESLint config changes are consistent with the eslint-plugin-react-hooks ^6.1.1 upgrade: default import,
recommended-latestconfig, and new rules (unsupported-syntax,incompatible-library).Also applies to: 9-10, 18-19
25-25: LGTM!Disabling
@eslint-react/dom/no-missing-button-typein test files is reasonable, as test code often uses simplified button elements without explicit types.packages/react-query-persist-client/eslint.config.js (2)
4-4: Default ES import is correct foreslint-plugin-react-hooks
The plugin’smainentry (index.js) exports via CommonJS, soimport reactHooks from 'eslint-plugin-react-hooks'works as expected.
18-19: Plugin v6.1.1 includes both rules The rules react-hooks/unsupported-syntax and react-hooks/incompatible-library are valid in eslint-plugin-react-hooks@^6.1.1.
| // @ts-expect-error wtf | ||
| ...reactHooks.configs['recommended-latest'], |
There was a problem hiding this comment.
Replace unprofessional comment with a clear explanation.
The comment "wtf" is unprofessional and doesn't explain why the type error is being suppressed. Replace it with a clear explanation of the type issue.
- // @ts-expect-error wtf
+ // @ts-expect-error - recommended-latest config has incomplete type definitions
...reactHooks.configs['recommended-latest'],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @ts-expect-error wtf | |
| ...reactHooks.configs['recommended-latest'], | |
| // @ts-expect-error - recommended-latest config has incomplete type definitions | |
| ...reactHooks.configs['recommended-latest'], |
🤖 Prompt for AI Agents
In packages/react-query-persist-client/eslint.config.js around lines 9 to 10,
replace the unprofessional "// @ts-expect-error wtf" comment with a concise,
professional explanation of the suppression: state which type incompatibility or
declaration mismatch is happening (e.g., that
reactHooks.configs['recommended-latest'] has an incompatible or missing type
declaration for this project's ESLint config), why the suppression is necessary
(to accept the runtime shape despite TypeScript complaining), and include a TODO
linking to an issue or task to resolve the typing mismatch later; keep it short
and informative.
| const mounted = React.useRef<boolean>(null) | ||
| // this simulates a synchronous update between the time the query is created | ||
| // and the time it is subscribed to that could be missed otherwise | ||
| if (!mounted.current) { | ||
| if (mounted.current === null) { |
There was a problem hiding this comment.
Fix type annotation for null initialization.
The ref is typed as React.useRef<boolean> but initialized with null. This is a type mismatch that violates TypeScript's type safety.
Apply this diff to correct the type:
- const mounted = React.useRef<boolean>(null)
+ const mounted = React.useRef<boolean | null>(null)The usage pattern (initialize with null, then set to true) requires the union type boolean | null to properly represent all possible values the ref can hold.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mounted = React.useRef<boolean>(null) | |
| // this simulates a synchronous update between the time the query is created | |
| // and the time it is subscribed to that could be missed otherwise | |
| if (!mounted.current) { | |
| if (mounted.current === null) { | |
| const mounted = React.useRef<boolean | null>(null) | |
| // this simulates a synchronous update between the time the query is created | |
| // and the time it is subscribed to that could be missed otherwise | |
| if (mounted.current === null) { |
🤖 Prompt for AI Agents
In packages/react-query/src/__tests__/useQuery.test.tsx around lines 6165 to
6168, the ref is declared as React.useRef<boolean> but initialized with null
causing a TypeScript type mismatch; update the ref's generic type to include
null (boolean | null) so it can be initialized with null and later set to true,
e.g., change the declaration to useRef<boolean | null>(null).
| export default [ | ||
| ...rootConfig, | ||
| reactHooks.configs.recommended, | ||
| // @ts-expect-error wtf |
There was a problem hiding this comment.
drive-by comment as I saw your posts on Bluesky...
if you hadn't already come across it, thought it'd be helpful to point to facebook/react#34745, which explains (some of?) the type issues with integrating the plugin's config (and the expect error overrides you needed).
Issue was closed but the fix hasn't been published yet. You may have a better time with integration once it is 😄
There was a problem hiding this comment.
ts-expect-error should go away if the types become correct
what a chore
Summary by CodeRabbit
Bug Fixes
Chores