From 43c510435f1bbb4a26a92b16f83d6776d4a1417a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 23 Aug 2026 16:32:23 -0700 Subject: [PATCH] fix(react-query): close the lint's blind spots, and the drift they hid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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({ ... })` — 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. --- .../knowledge-base-selector.tsx | 6 +- apps/sim/hooks/use-permission-config.ts | 4 +- scripts/check-react-query-patterns.ts | 128 +++++++++++++++++- 3 files changed, 132 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-base-selector/knowledge-base-selector.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-base-selector/knowledge-base-selector.tsx index 73e4bedef42..521e70eb7ae 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-base-selector/knowledge-base-selector.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-base-selector/knowledge-base-selector.tsx @@ -14,7 +14,7 @@ import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflow import type { SubBlockConfig } from '@/blocks/types' import { useKnowledgeBasesList } from '@/hooks/kb/use-knowledge' import { useFolderMap } from '@/hooks/queries/folders' -import { fetchKnowledgeBase } from '@/hooks/queries/kb/knowledge' +import { fetchKnowledgeBase, KNOWLEDGE_BASE_DETAIL_STALE_TIME } from '@/hooks/queries/kb/knowledge' import { collectDuplicateNames, disambiguateLabelByFolder } from '@/hooks/queries/utils/folder-tree' import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys' @@ -75,9 +75,9 @@ export function KnowledgeBaseSelector({ const selectedKnowledgeBaseQueries = useQueries({ queries: selectedIds.map((selectedId) => ({ queryKey: knowledgeKeys.detail(selectedId), - queryFn: () => fetchKnowledgeBase(selectedId), + queryFn: ({ signal }: { signal: AbortSignal }) => fetchKnowledgeBase(selectedId, signal), enabled: Boolean(selectedId), - staleTime: 60 * 1000, + staleTime: KNOWLEDGE_BASE_DETAIL_STALE_TIME, })), }) diff --git a/apps/sim/hooks/use-permission-config.ts b/apps/sim/hooks/use-permission-config.ts index 0b1cb308568..b6740a30fb0 100644 --- a/apps/sim/hooks/use-permission-config.ts +++ b/apps/sim/hooks/use-permission-config.ts @@ -50,11 +50,13 @@ const allowedIntegrationsKeys = { env: () => [...allowedIntegrationsKeys.all, 'env'] as const, } +export const ALLOWED_INTEGRATIONS_STALE_TIME = 5 * 60 * 1000 + function useAllowedIntegrationsFromEnv() { return useQuery({ queryKey: allowedIntegrationsKeys.env(), queryFn: ({ signal }) => requestJson(getAllowedIntegrationsContract, { signal }), - staleTime: 5 * 60 * 1000, + staleTime: ALLOWED_INTEGRATIONS_STALE_TIME, }) } diff --git a/scripts/check-react-query-patterns.ts b/scripts/check-react-query-patterns.ts index df85744fcb0..86f8525b6b6 100644 --- a/scripts/check-react-query-patterns.ts +++ b/scripts/check-react-query-patterns.ts @@ -8,6 +8,10 @@ * * 1. missing-stale-time — useQuery/useInfiniteQuery/useSuspenseQuery without an explicit `staleTime` * 2. queryfn-no-signal — an inline `queryFn` that takes no args (cannot forward the AbortSignal) + * 2b. stale-time-literal — `staleTime` given as a numeric literal rather than a named constant, so a + * server prefetch hydrating the same key cannot import the one value and the + * two drift apart silently. `0` is exempt: it is a sentinel meaning "always + * refetch", not a duration anyone tunes. * 3. inline-query-key — `queryKey: ['literal', ...]` instead of a colocated key factory * 4. key-factory-no-root — a `*Keys` factory in hooks/queries/** without an `all` root key * 5. key-fetch-arg-drift — an identifier the queryFn forwards into the fetch (e.g. `workspaceId`) @@ -44,6 +48,7 @@ const ALLOW = 'rq-lint-allow:' type Category = | 'missing-stale-time' + | 'stale-time-literal' | 'queryfn-no-signal' | 'inline-query-key' | 'key-factory-no-root' @@ -60,6 +65,9 @@ interface Violation { const SUGGESTION: Record = { 'missing-stale-time': 'add an explicit staleTime (default 0 is rarely correct); e.g. staleTime: 60 * 1000', + 'stale-time-literal': + 'assign staleTime from a named exported constant (e.g. ENTITY_LIST_STALE_TIME) so a server ' + + 'prefetch on the same query key can import the one value instead of restating it', 'queryfn-no-signal': 'destructure the AbortSignal: queryFn: ({ signal }) => fetchX(..., signal) and forward it', 'inline-query-key': @@ -131,7 +139,75 @@ function hasAllow(lines: string[], line: number): boolean { return false } -const QUERY_CALL = /\b(useQuery|useInfiniteQuery|useSuspenseQuery|useSuspenseInfiniteQuery)\s*\(/g +/** + * The optional explicit type argument on a query call — `useQuery({ ... })`. + * + * Matched rather than ignored because a call carrying one is still a query call: without this + * the scan skipped every generically-typed query, including ten in the strict zone, which then + * reported zero violations while never having looked at them. One level of nesting is enough + * for the shapes that occur here (`useQuery>`). + */ +const TYPE_ARGS = String.raw`(?:\s*<[^<>()]*(?:<[^<>()]*>[^<>()]*)*>)?` + +const QUERY_CALL = new RegExp( + String.raw`\b(useQuery|useInfiniteQuery|useSuspenseQuery|useSuspenseInfiniteQuery)${TYPE_ARGS}\s*\(`, + 'g' +) + +/** + * `useQueries` nests its option objects one level deeper — `useQueries({ queries: [ {...} ] })` — + * so the single-object walk above would read the wrapper and see one `staleTime` anywhere inside + * the array as covering every entry. It gets its own pass that visits each entry. + */ +const USE_QUERIES_CALL = new RegExp(String.raw`\buseQueries${TYPE_ARGS}\s*\(`, 'g') + +/** + * Every top-level `{ ... }` inside a `queries` value, whether written as an array literal or + * produced by a `.map(...)` callback — both forms put each query's options in a brace group at + * the same nesting depth, so one balanced scan covers them. + */ +function splitObjectLiterals(value: string): string[] { + const out: string[] = [] + let depth = 0 + let start = -1 + let inStr: string | null = null + for (let i = 0; i < value.length; i++) { + const c = value[i] + if (inStr) { + if (c === inStr && value[i - 1] !== '\\') inStr = null + continue + } + if (c === '"' || c === "'" || c === '`') { + inStr = c + continue + } + if (c === '{') { + if (depth === 0) start = i + depth++ + continue + } + if (c === '}') { + depth-- + if (depth === 0 && start !== -1) { + out.push(value.slice(start, i + 1)) + start = -1 + } + } + } + return out +} + +/** + * Whether a `staleTime` value is a named constant rather than a literal duration. + * + * `0` is allowed: it is the sentinel for "always refetch", not a number anyone tunes, and the + * drift this rule prevents cannot occur when there is no window to keep in step. + */ +function isNamedStaleTime(value: string): boolean { + const trimmed = value.trim() + if (trimmed === '0') return true + return !/^[0-9]/.test(trimmed) +} const QUERYFN_NOARG = /queryFn\s*:\s*(?:async\s+)?\(\s*\)\s*=>/ const QUERYFN_PRESENT = /queryFn\s*:/ const INLINE_KEY = /queryKey\s*:\s*\[\s*[`'"]/ @@ -278,6 +354,14 @@ function scanFile(rel: string, content: string): Violation[] { if (!/\bstaleTime\b/.test(obj) && !/\.\.\.\w/.test(obj)) { add(m.index, 'missing-stale-time', `${m[1]}({ ... }) without staleTime`) } + const staleTimeValue = extractOptionValue(obj, 'staleTime') + if (staleTimeValue !== null && !isNamedStaleTime(staleTimeValue)) { + add( + m.index, + 'stale-time-literal', + `${m[1]} staleTime is the literal ${staleTimeValue.trim()}` + ) + } if (QUERYFN_PRESENT.test(obj) && QUERYFN_NOARG.test(obj)) { add(m.index, 'queryfn-no-signal', `${m[1]} queryFn takes no args`) } @@ -290,6 +374,44 @@ function scanFile(rel: string, content: string): Violation[] { } } + // 2b: useQueries — same checks, applied to each entry of its `queries` array + USE_QUERIES_CALL.lastIndex = 0 + let q: RegExpExecArray | null = USE_QUERIES_CALL.exec(content) + for (; q !== null; q = USE_QUERIES_CALL.exec(content)) { + const parenStart = q.index + q[0].length - 1 + const arg = matchBalanced(content, parenStart, '(', ')') + const braceRel = arg.indexOf('{') + if (braceRel === -1) continue + const wrapper = matchBalanced(arg, braceRel, '{', '}') + const queriesValue = extractOptionValue(wrapper, 'queries') + if (queriesValue === null) continue + + for (const entry of splitObjectLiterals(queriesValue)) { + if (/\.\.\.\w/.test(entry)) continue + if (!/\bstaleTime\b/.test(entry)) { + add(q.index, 'missing-stale-time', 'useQueries entry without staleTime') + } + const entryStaleTime = extractOptionValue(entry, 'staleTime') + if (entryStaleTime !== null && !isNamedStaleTime(entryStaleTime)) { + add( + q.index, + 'stale-time-literal', + `useQueries entry staleTime is the literal ${entryStaleTime.trim()}` + ) + } + if (QUERYFN_PRESENT.test(entry) && QUERYFN_NOARG.test(entry)) { + add(q.index, 'queryfn-no-signal', 'useQueries entry queryFn takes no args') + } + for (const id of findKeyFetchArgDrift(entry)) { + add( + q.index, + 'key-fetch-arg-drift', + `useQueries: '${id}' passed to fetch but absent from queryKey` + ) + } + } + } + // 3: inline query keys for (let i = 0; i < lines.length; i++) { if (INLINE_KEY.test(lines[i])) { @@ -345,7 +467,9 @@ async function main() { for (const file of files) { const rel = path.relative(ROOT, file) const content = await readFile(file, 'utf8') - if (!/\buse(Query|InfiniteQuery|SuspenseQuery|Mutation)\b|[kK]eys\s*[:=]/.test(content)) + /* `useQueries` must be listed before `useQuery`: the alternation is ordered, and a trailing + `\b` after `useQuery` would reject it outright on the `s`. */ + if (!/\buse(Queries|Query|InfiniteQuery|SuspenseQuery|Mutation)\b|[kK]eys\s*[:=]/.test(content)) continue all.push(...scanFile(rel, content)) }