Skip to content

feat(cache): add allowQuery option to filter query params in cache key #4078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fmoessle wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(cache): add allowQuery option to filter query params in cache key #4078

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61e4c3f
feat(cache): add `allowQuery` option to filter query params in cache key
fmoessle Mar 6, 2026
c0fd5d5
fix: accept `readonly string[]` for `allowQuery` type
fmoessle Mar 6, 2026
3e54b26
test: update Vercel preset snapshots for allowQuery
fmoessle Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/1.docs/7.cache.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ The `defineCachedHandler` and `defineCachedFunction` functions accept the follow
::field{name="varies" type="string[]"}
An array of request headers to be considered for the cache, [learn more](https://github.com/nitrojs/nitro/issues/1031). If utilizing in a multi-tenant environment, you may want to pass `['host', 'x-forwarded-host']` to ensure these headers are not discarded and that the cache is unique per tenant.
::
::field{name="allowQuery" type="string[]"}
List of query parameter names to include in the cache key. If `undefined`, all query parameters are included. If an empty array `[]`, all query parameters are ignored (only the pathname is used for caching). Only applicable to cached event handlers. :br
Defaults to `undefined`.
::
::

## Cache keys and invalidation
Expand Down
15 changes: 14 additions & 1 deletion src/runtime/internal/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,20 @@ export function defineCachedHandler(
return escapeKey(customKey);
}
// Auto-generated key
const _path = event.url.pathname + event.url.search;
let _path: string;
if (opts.allowQuery) {
const params = new URLSearchParams();
for (const key of opts.allowQuery) {
const value = event.url.searchParams.get(key);
if (value !== null) {
params.set(key, value);
}
}
const search = params.size > 0 ? `?${params.toString()}` : "";
Comment on lines +205 to +213
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve repeated allowed query values in the cache key.

Line 208 only reads the first value with searchParams.get(), so ?tag=a&tag=b and ?tag=a collapse to the same cache entry when tag is allowed. That can serve the wrong cached response for multi-value filters.

🐛 Proposed fix 
         const params = new URLSearchParams();
         for (const key of opts.allowQuery) {
-          const value = event.url.searchParams.get(key);
-          if (value !== null) {
-            params.set(key, value);
+          for (const value of event.url.searchParams.getAll(key)) {
+            params.append(key, value);
           }
         }
📝 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.

Suggested change
if (opts.allowQuery) {
const params = new URLSearchParams();
for (const key of opts.allowQuery) {
const value = event.url.searchParams.get(key);
if (value !== null) {
params.set(key, value);
}
}
const search = params.size > 0 ? `?${params.toString()}` : "";
if (opts.allowQuery) {
const params = new URLSearchParams();
for (const key of opts.allowQuery) {
for (const value of event.url.searchParams.getAll(key)) {
params.append(key, value);
}
}
const search = params.size > 0 ? `?${params.toString()}` : "";
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/internal/cache.ts` around lines 205 - 213, The cache key builder
currently collapses repeated query values because it uses
event.url.searchParams.get() and params.set(), so e.g. ?tag=a&tag=b and ?tag=a
become identical; change the logic in the block that references opts.allowQuery
and event.url.searchParams to retrieve all values for each allowed key (use
getAll or iterate searchParams entries for that key) and append each value to
the local URLSearchParams (use params.append) so duplicate values and their
order are preserved when computing the search string assigned to the search
variable.

_path = event.url.pathname + search;
} else {
_path = event.url.pathname + event.url.search;
}
let _pathname: string;
try {
_pathname = escapeKey(decodeURI(parseURL(_path).pathname)).slice(0, 16) || "index";
Expand Down
7 changes: 7 additions & 0 deletions src/types/runtime/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,11 @@ export interface CachedEventHandlerOptions extends Omit<
> {
headersOnly?: boolean;
varies?: string[] | readonly string[];
/**
* List of query string parameter names that will be considered for caching.
* - If undefined, all query parameters are included in the cache key.
* - If an empty array `[]`, all query parameters are ignored (only pathname is used for caching).
* - If a list of parameter names, only those parameters are included in the cache key.
*/
allowQuery?: string[] | readonly string[];
Comment on lines +41 to +47
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify how `allowQuery` values are incorporated into the cache key.
# Expected result: the implementation uses `searchParams.get(key)`,
# which confirms only one value per query name is considered.

sed -n '201,216p' src/runtime/internal/cache.ts

Repository: nitrojs/nitro

Length of output: 607

JSDoc must clarify that only the first value per query parameter is used in the cache key.

The implementation uses event.url.searchParams.get(key), which reads only the first value when a query parameter appears multiple times. Requests like /foo?q=a&q=b and /foo?q=a will collide when allowQuery: ["q"] because both produce the same cache key. Update the JSDoc on lines 43-44 to explicitly state that only the first occurrence of each allowed parameter contributes to the key, or modify the implementation to include all occurrences. Per coding guidelines, JSDoc must accurately reflect API behavior.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/types/runtime/cache.ts` around lines 41 - 47, The JSDoc for the
allowQuery field is inaccurate: the cache key currently uses
event.url.searchParams.get(key) which only reads the first value for repeated
query parameters, so repeated params (e.g., q=a&q=b) will collide with single
occurrences; update the allowQuery JSDoc comment (for the allowQuery property)
to explicitly state "only the first occurrence/value of each allowed query
parameter is used when building the cache key" OR change the implementation that
builds the cache key to use all occurrences (e.g., searchParams.getAll or
iterate entries) so every value contributes to the key — reference allowQuery
and the code path using event.url.searchParams.get(key) to locate where to
update either the docs or the key-construction logic.

}
1 change: 1 addition & 0 deletions test/fixture/nitro.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export default defineConfig({
},
"/rules/dynamic": { cache: false, isr: false },
"/rules/redirect": { redirect: "/base" },
"/rules/allow-query/**": { cache: { swr: true, maxAge: 60, allowQuery: ["q"] } },
"/rules/isr/**": { isr: { allowQuery: ["q"] } },
"/rules/isr-ttl/**": { isr: 60 },
"/rules/swr/**": { swr: true },
Expand Down
10 changes: 10 additions & 0 deletions test/fixture/server/routes/api/cached-allow-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { defineCachedHandler } from "nitro/cache";

export default defineCachedHandler(
(event) => {
return {
timestamp: Date.now(),
};
},
{ swr: true, maxAge: 60, allowQuery: ["q"] }
);
21 changes: 21 additions & 0 deletions test/tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,27 @@ export function testNitro(
}
}
);

it.skipIf(ctx.isIsolated || (isWindows && ctx.preset === "nitro-dev"))(
"allowQuery should ignore unlisted query params in cache key",
async () => {
const { data: first } = await callHandler({
url: "/api/cached-allow-query?q=search&utm_source=email",
});

// Same q param, different unlisted param should hit cache
const { data: second } = await callHandler({
url: "/api/cached-allow-query?q=search&utm_source=twitter",
});
expect(second.timestamp).toBe(first.timestamp);

// Different q param should get a different cache entry
const { data: third } = await callHandler({
url: "/api/cached-allow-query?q=other&utm_source=email",
});
expect(third.timestamp).not.toBe(first.timestamp);
}
);
});

describe("scanned files", () => {
Expand Down
Loading