fix(react-query): close the lint's blind spots, and the drift they hid - #7020
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Adds a Reviewed by Cursor Bugbot for commit 132b178. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 132b178. Configure here.
| if (queriesValue === null) continue | ||
|
|
||
| for (const entry of splitObjectLiterals(queriesValue)) { | ||
| if (/\.\.\.\w/.test(entry)) continue |
There was a problem hiding this comment.
useQueries spread skips entries
Medium Severity
The new useQueries pass does continue on any entry matching /\.\.\.\w/, which also hits normal array spreads in queryKey and object spreads in queryFn args. That skips every check for those entries. The single-query path only softens missing-stale-time when options are spread, so this reopens a blind spot—including the existing useQueries site in use-selector-query.ts.
Reviewed by Cursor Bugbot for commit 132b178. Configure here.
Greptile SummaryThis PR aligns two React Query callers with shared stale-time constants, forwards cancellation into knowledge-base fetches, and expands the React Query audit to inspect generically typed hooks and individual
Confidence Score: 4/5The PR appears safe to merge, with non-blocking gaps remaining in how comprehensively the expanded lint gate recognizes generic calls and exported stale-time constants. The application changes preserve query behavior while improving cancellation and freshness consistency; the remaining concerns affect the audit’s ability to enforce those conventions across additional valid source forms. Files Needing Attention: scripts/check-react-query-patterns.ts
|
| Filename | Overview |
|---|---|
| scripts/check-react-query-patterns.ts | Expands static query-pattern coverage, but the generic and stale-time classifiers retain specific syntax and enforcement blind spots. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-base-selector/knowledge-base-selector.tsx | Correctly reuses the shared detail stale time and propagates query cancellation. |
| apps/sim/hooks/use-permission-config.ts | Extracts the inline freshness duration into an exported reusable constant. |
Reviews (1): Last reviewed commit: "fix(react-query): close the lint's blind..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * reported zero violations while never having looked at them. One level of nesting is enough | ||
| * for the shapes that occur here (`useQuery<Record<string, T>>`). | ||
| */ | ||
| const TYPE_ARGS = String.raw`(?:\s*<[^<>()]*(?:<[^<>()]*>[^<>()]*)*>)?` |
There was a problem hiding this comment.
TYPE_ARGS rejects parentheses and supports only one nested angle-bracket level, so valid query generics containing function types, parenthesized conditional types, or deeper nested generics remain unscanned and can bypass the React Query checks.
| */ | ||
| function isNamedStaleTime(value: string): boolean { | ||
| const trimmed = value.trim() | ||
| if (trimmed === '0') return true | ||
| return !/^[0-9]/.test(trimmed) |
There was a problem hiding this comment.
isNamedStaleTime accepts every expression not beginning with a digit, including local identifiers, parenthesized literals, and unary numeric expressions. The audit therefore accepts values that are not named exported constants and cannot provide the intended shared freshness source for prefetch callers.
34528af to
e11ae5d
Compare
`check-react-query-patterns.ts` reported a clean strict zone while never
looking at part of it. Two gaps in one regex:
`\buseQuery\s*\(` does not match `useQuery<Row[]>({ ... })` — a type argument
sits between the name and the paren. Twenty query calls carry one, ten of them
inside the zero-tolerance zone, so that zone's "0 violations" was partly a
statement about what the scan could see.
`useQueries` was absent from both the call pattern and the file pre-filter,
where `\buse(Query|...)\b` rejects it on the trailing `s`. All sixteen call
sites were unscanned, and its options nest one level deeper — inside a
`queries` array — so it needs its own pass per entry rather than one that reads
the wrapper and takes a single `staleTime` anywhere inside as covering them all.
With both closed, three real violations surfaced:
- `knowledge-base-selector` served `knowledgeKeys.detail(id)` with an inline
`60 * 1000` while `useKnowledgeBaseQuery` serves the same cache key from
`KNOWLEDGE_BASE_DETAIL_STALE_TIME`. The two agree only by coincidence, and
TanStack resolves staleTime per observer, so tuning the constant would have
left this component on the old window for the same entry.
- The same call dropped the `AbortSignal`, which `fetchKnowledgeBase` accepts.
- `use-permission-config` gave `staleTime` as a literal with no named constant.
The new `stale-time-literal` category makes the second half of the CLAUDE.md
rule enforceable — it required a named constant, and only the presence of
`staleTime` was ever checked. `0` is exempt: it is the sentinel for "always
refetch", not a window anyone keeps in step with a prefetch.
Verified the new rules can fail by reverting each fix and watching the audit
report it, then restoring.
132b178 to
43c5104
Compare
1 participant
The gate was partly reporting on what it could see
scripts/check-react-query-patterns.tshas a zero-tolerance strict zone (apps/sim/hooks/queries/**) and an empty baseline, and it passes clean. Two gaps in one regex explain part of that.1. Type arguments.
\buseQuery\s*\(does not matchuseQuery<Row[]>({ ... })— the type argument sits between the name and the paren. 20 query calls carry one, 10 inside the strict zone. That zone's "0 violations" was, for those ten, a statement about the scan rather than the code.2.
useQueries. Absent from both the call pattern and the file pre-filter, where\buse(Query|...)\brejects it on the trailings. All 16 call sites were unscanned. It also nests its options one level deeper, inside aqueriesarray — so a naive fix that reads the wrapper would take a singlestaleTimeanywhere inside as covering every entry. It gets its own per-entry pass.Closing both surfaced exactly three violations.
The drift they hid
knowledge-base-selector.tsxservedknowledgeKeys.detail(id)with an inline60 * 1000, whileuseKnowledgeBaseQueryserves the same cache key fromKNOWLEDGE_BASE_DETAIL_STALE_TIME. They agree today by coincidence. TanStack resolvesstaleTimeper observer, so tuning the constant would have left this component on the old freshness window for the same cache entry — the precise failure the CLAUDE.md rule exists to prevent.The same call also dropped the
AbortSignalthatfetchKnowledgeBaseaccepts, anduse-permission-config.tsgavestaleTimeas a bare literal.New category:
stale-time-literalThe rule reads "assigned from a named exported constant, never an inline numeric literal" — but only the presence of
staleTimewas ever checked. That is the half that let all three through, so it is now enforced.0is exempt. It is the sentinel for "always refetch", not a window anyone keeps in step with a prefetch, and the two documentedstaleTime: 0sites in the strict zone are correct as written.Verified the rules can fail
Reverted each of the three fixes, confirmed the audit reported it, then restored:
With the fixes in place both zones are 0, so no baseline entries were needed.
Testing
hooksandapp/workspacebun run type-checkclean; the audit passes under--checkFollow-up not included
use-permission-config.tsalso breaks "all React Query hooks live inhooks/queries/" — its private key factory and hook sit outside the strict zone, which is why the zone never saw them. Moving the file would put it under strict enforcement but touches imports across the permission surface, so it wants its own PR.