diff --git a/eslint-plugins/healthlog/__tests__/queryKey-factory.test.mjs b/eslint-plugins/healthlog/__tests__/queryKey-factory.test.mjs index 365107dc2..d352bf4a4 100644 --- a/eslint-plugins/healthlog/__tests__/queryKey-factory.test.mjs +++ b/eslint-plugins/healthlog/__tests__/queryKey-factory.test.mjs @@ -34,9 +34,32 @@ ruleTester.run("queryKey-factory", rule, { code: 'useQuery({ queryKey: ["fixture"] });', filename: "/repo/src/components/__tests__/example.test.tsx", }, + // A server module outside every guarded root stays unguarded — the roots + // are the scope decision, not an accident of this fixture. { - code: 'useQuery({ queryKey: ["server-prefetch"] });', - filename: "/repo/src/app/server-page.tsx", + code: 'useQuery({ queryKey: ["server-only-lib"] });', + filename: "/repo/src/lib/reports/build-report.ts", + }, + // The positional cache API, used correctly: the key still comes from the + // factory, so the shape that poisons the cache never arises. + { + code: "queryClient.setQueryData(queryKeys.example(id), next);", + filename: "/repo/src/app/page.tsx", + }, + { + code: "const key = queryKeys.example(id); queryClient.setQueryData(key, next);", + filename: "/repo/src/components/example.tsx", + }, + // Tests seed caches with literal tuples on purpose; the factory directory + // constructs them by definition. Both stay exempt on the positional form + // exactly as they are on the object-property form. + { + code: 'client.setQueryData(["fixture"], 1);', + filename: "/repo/src/hooks/__tests__/use-example.test.ts", + }, + { + code: 'client.setQueryData(["fixture"], 1);', + filename: "/repo/src/lib/query-keys/__fixtures__/seed.ts", }, ], invalid: [ @@ -60,5 +83,65 @@ ruleTester.run("queryKey-factory", rule, { filename: "/repo/src/lib/client-state.tsx", errors: [{ messageId: "bareArray" }], }, + // src/app is where the server prefetch pages build the keys the client + // then reads. A key that differs from the client's is a cache miss at + // best and a poisoned cell at worst, and without `"use client"` on the + // page the rule used to skip the file entirely. + { + code: 'useQuery({ queryKey: ["server-prefetch"] });', + filename: "/repo/src/app/server-page.tsx", + errors: [{ messageId: "bareArray" }], + }, + { + code: 'queryClient.prefetchQuery({ queryKey: ["dashboard"], queryFn: load });', + filename: "/repo/src/app/coach/page.tsx", + errors: [{ messageId: "bareArray" }], + }, + // The positional form of the same mistake. `setQueryData` seeds a cache + // cell by key exactly as `queryKey:` reads one, so a bare array here is + // the same defect written differently. + { + code: 'queryClient.setQueryData(["bare", "positional"], 1);', + filename: "/repo/src/app/coach/page.tsx", + errors: [{ messageId: "positionalArray" }], + }, + { + code: 'const cached = queryClient.getQueryData(["bare"]);', + filename: "/repo/src/hooks/use-example.ts", + errors: [{ messageId: "positionalArray" }], + }, + { + code: 'queryClient.getQueryState(["bare"]);', + filename: "/repo/src/components/example.tsx", + errors: [{ messageId: "positionalArray" }], + }, + { + code: 'queryClient.setQueryDefaults(["bare"], { staleTime: 0 });', + filename: "/repo/src/lib/queries/use-example.ts", + errors: [{ messageId: "positionalArray" }], + }, + { + code: 'queryClient.setMutationDefaults(["bare"], { retry: 0 });', + filename: "/repo/src/components/example.tsx", + errors: [{ messageId: "positionalArray" }], + }, + // Destructured off the client, and reached through a computed member — + // both spell the same call and both must be seen. + { + code: 'const { setQueryData } = useQueryClient(); setQueryData(["bare"], 1);', + filename: "/repo/src/components/example.tsx", + errors: [{ messageId: "positionalArray" }], + }, + { + code: 'queryClient["setQueryData"](["bare"], 1);', + filename: "/repo/src/components/example.tsx", + errors: [{ messageId: "positionalArray" }], + }, + // Both holes in one file, which is the probe that found them. + { + code: 'queryClient.setQueryData(["bare", "positional"], 1); useQuery({ queryKey: ["bare", "object-property"] });', + filename: "/repo/src/app/coach/page.tsx", + errors: [{ messageId: "positionalArray" }, { messageId: "bareArray" }], + }, ], }); diff --git a/eslint-plugins/healthlog/queryKey-factory.js b/eslint-plugins/healthlog/queryKey-factory.js index 3047084eb..548cb44c5 100644 --- a/eslint-plugins/healthlog/queryKey-factory.js +++ b/eslint-plugins/healthlog/queryKey-factory.js @@ -2,16 +2,39 @@ * @fileoverview ESLint rule — `queryKey` / `mutationKey` literal-array * factory enforcement across the complete client tree. * - * Every TanStack key must come from `src/lib/query-keys/`. The rule flags - * object-property `queryKey: [ … ]` and `mutationKey: [ … ]` values in - * components, hooks, client query modules, and any source module declaring - * top-level `"use client"`. + * Every TanStack key must come from `src/lib/query-keys/`. The rule flags a + * bare array wherever a key is supplied, in either of the two shapes TanStack + * accepts: + * + * - the object property, `{ queryKey: [ … ] }` / `{ mutationKey: [ … ] }`; + * - the FIRST POSITIONAL argument of the QueryClient methods that take a key + * directly, `setQueryData([ … ], value)`, `getQueryData([ … ])`, and their + * siblings below. + * + * The positional shape was invisible until 2026-09-03. `setQueryData` seeds + * the same cache cell that `queryKey:` reads, so a literal there is the same + * defect written differently — and a probe placing one in a component + * produced no error at all. + * + * Enforced roots: `src/components/`, `src/hooks/`, `src/lib/queries/`, + * `src/app/`, and any source module declaring top-level `"use client"`. + * `src/app/` joined the list on the same date: the server prefetch pages there + * build the keys the client subsequently reads, and a server key that differs + * from the client's is a cache miss at best and a poisoned cell at worst. They + * carry no `"use client"` directive, so nothing guarded them before. * * Accepted call-site shapes include direct factory calls, identifiers already * built from the factory, and conditionals selecting factory calls. Tests and * the factory definition directory are exempt because they intentionally * construct literal fixtures and tuples. * + * Limits, so a green run is not read as more than it is: the positional check + * matches by METHOD NAME, so a key handed to a helper of the project's own + * naming (`seedCache(["…"], v)`) is not seen, and conversely an unrelated + * object with a `setQueryData` method would be flagged. A key assembled into a + * variable before the call site (`const k = ["a", id]`) is invisible to both + * checks — only the literal at the key position is matched. + * * @see src/lib/query-keys/ * @see src/lib/__tests__/query-keys.test.ts */ @@ -22,7 +45,28 @@ const FACTORY_HOME_DIRECTORY = "src/lib/query-keys"; // Components and hooks are client-facing by convention. Query modules are // client-transitive even if a future refactor moves the directive to a barrel. -const CLIENT_ROOTS = ["src/components/", "src/hooks/", "src/lib/queries/"]; +// `src/app/` is where the server prefetch pages seed the client's cache, so +// their keys have to come from the same factory the client reads from. +const GUARDED_ROOTS = [ + "src/components/", + "src/hooks/", + "src/lib/queries/", + "src/app/", +]; + +// QueryClient methods whose FIRST argument is a query key or mutation key, +// supplied positionally rather than inside an options object. Everything else +// on the client (`invalidateQueries`, `prefetchQuery`, …) takes a filters or +// options object and is covered by the `queryKey:` property check. +const POSITIONAL_KEY_METHODS = new Set([ + "setQueryData", + "getQueryData", + "getQueryState", + "setQueryDefaults", + "getQueryDefaults", + "setMutationDefaults", + "getMutationDefaults", +]); function toPosix(filename) { return filename.replace(/\\/g, "/"); @@ -46,7 +90,7 @@ function isGuarded(filename, sourceCode) { return false; } if (isTestFile(posix)) return false; - if (CLIENT_ROOTS.some((root) => posix.includes(root))) return true; + if (GUARDED_ROOTS.some((root) => posix.includes(root))) return true; return sourceCode.ast.body.some( (statement) => statement.type === "ExpressionStatement" && @@ -54,6 +98,25 @@ function isGuarded(filename, sourceCode) { ); } +/** + * The called name, whether the call is bare (`setQueryData(…)`, destructured + * off the client), a member (`qc.setQueryData(…)`), or a static computed + * member (`qc["setQueryData"](…)`). + */ +function calleeName(callee) { + if (callee.type === "Identifier") return callee.name; + if (callee.type !== "MemberExpression") return null; + if (!callee.computed && callee.property.type === "Identifier") { + return callee.property.name; + } + if (callee.computed && callee.property.type === "Literal") { + return typeof callee.property.value === "string" + ? callee.property.value + : null; + } + return null; +} + /** @type {import('eslint').Rule.RuleModule} */ const queryKeyFactoryRule = { meta: { @@ -66,6 +129,8 @@ const queryKeyFactoryRule = { messages: { bareArray: "Bare-array `{{prop}}: [ … ]` bypasses the queryKeys factory. Import from `@/lib/query-keys` and call `queryKeys.()` so cache-invalidation bundles stay in lockstep.", + positionalArray: + "Bare-array key passed to `{{call}}( … )` bypasses the queryKeys factory. It seeds or reads the same cache cell as a `queryKey`, so import from `@/lib/query-keys` and call `queryKeys.()` instead.", }, schema: [], }, @@ -76,7 +141,7 @@ const queryKeyFactoryRule = { return {}; } - function check(node) { + function checkProperty(node) { // `node` is a Property like `queryKey: [ … ]`. Only flag the // bare ArrayExpression form — identifiers, call expressions, // conditional expressions, etc. are all acceptable indirection @@ -98,8 +163,22 @@ const queryKeyFactoryRule = { }); } + function checkPositional(node) { + const name = calleeName(node.callee); + if (!name || !POSITIONAL_KEY_METHODS.has(name)) return; + const first = node.arguments[0]; + if (!first || first.type !== "ArrayExpression") return; + + context.report({ + node: first, + messageId: "positionalArray", + data: { call: name }, + }); + } + return { - Property: check, + Property: checkProperty, + CallExpression: checkPositional, }; }, };