fix(eslint-plugin-query): only match own properties in no-unstable-deps - #11180
fix(eslint-plugin-query): only match own properties in no-unstable-deps#11180yogesh968 wants to merge 1 commit into
Conversation
The rule keeps three plain objects keyed by identifier name: tracked custom hooks, tracked variables and React hook aliases. All three were read with a bare index access or the 'in' operator, so names inherited from 'Object.prototype' resolved to a prototype member instead of 'undefined'. Code using an identifier called 'toString', 'constructor' or 'valueOf' was therefore treated as a tracked query hook. A dependency named 'toString' got reported, and a call such as 'constructor(fn, [query])' was treated as a React hook invocation. Guard the three lookups with 'Object.hasOwn'.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Changesno-unstable-deps identifier tracking
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This localized change prevents prototype-inherited names from being treated as configured hooks or variables, with no actionable merge-blocking risk remaining after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixes #11118
The problem
no-unstable-depskeeps three plain objects keyed by identifier name, and reads all three without checking that the key is an own property:Names inherited from
Object.prototypetherefore resolve to a prototype member instead ofundefined, and unrelated code gets reported. #11118 covers thetrackedCustomHookslookup. The other two produce their own false positives:toString,constructororvalueOfis reported as the result of a query hook, with the inherited function interpolated into the messageconstructor(fn, [query])is treated as a React hook invocation, and its second argument is checked as a dependency arrayThe
toStringcase is the one from the issue's StackBlitz, wheretoStringcomes from Lodash.The change
Guard all three lookups with
Object.hasOwn, which is what the issue suggests.Tests
Three cases added to
no-unstable-deps.test.ts, one per lookup. All three fail onmainand pass here. Rule suite is green: 246 tests.Note
There are two open PRs on this issue, #11117 and #11125, and both fix the
trackedCustomHookslookup only. I ran into the other two lookups while checking whether that was the whole story, so this covers all three. Happy to rebase on either of them, or to close this if you would rather take one of those and have the remaining two handled as a follow up.Summary by CodeRabbit
Bug Fixes
no-unstable-depsincorrectly recognizing inherited names such astoStringandconstructoras React hooks, custom hooks, or tracked dependencies.Tests