Skip to content
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.

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

fix: apply cached rules to overlaping wildcard patterns #906

Merged
merged 8 commits into from
Feb 7, 2023
Merged
69 changes: 63 additions & 6 deletions src/rollup/plugins/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import { hash } from "ohash";
import type { Nitro } from "../../types";
import type { Nitro, NitroRouteRules, NitroEventHandler } from "../../types";
import { virtual } from "./virtual";

const unique = (arr: any[]) => [...new Set(arr)];
const getImportId = (p: string, lazy?: boolean) =>
(lazy ? "_lazy_" : "_") + hash(p).slice(0, 6);

export function handlers(nitro: Nitro) {
return virtual(
{
"#internal/nitro/virtual/server-handlers": () => {
const handlers = [...nitro.scannedHandlers, ...nitro.options.handlers];
const handlers: NitroEventHandler[] = [
...nitro.scannedHandlers,
...nitro.options.handlers,
];
if (nitro.options.serveStatic) {
handlers.unshift({
middleware: true,
Expand All @@ -25,6 +24,9 @@ export function handlers(nitro: Nitro) {
});
}

// If this handler would render a cached route rule then we can also inject a cached event handler
extendMiddlewareWithRuleOverlaps(handlers, nitro.options.routeRules);

// Imports take priority
const imports = unique(
handlers.filter((h) => !h.lazy).map((h) => h.handler)
Expand Down Expand Up @@ -68,3 +70,58 @@ ${handlers
nitro.vfs
);
}

function unique(arr: any[]) {
return [...new Set(arr)];
}

function getImportId(p: string, lazy?: boolean) {
return (lazy ? "_lazy_" : "_") + hash(p).slice(0, 6);
}

const WILDCARD_PATH_RE = /\/\*\*.*$/;

function extendMiddlewareWithRuleOverlaps(
handlers: NitroEventHandler[],
routeRules: Record<string, NitroRouteRules>
) {
const rules = Object.entries(routeRules);
for (const [path, rule] of rules) {
// We can ignore this rule if it is not cached and it isn't nested in a cached route
if (!rule.cache) {
// If we are nested 'within' a cached route, we want to inject a non-cached event handler
const isNested = rules.some(
([p, r]) =>
r.cache &&
WILDCARD_PATH_RE.test(p) &&
path.startsWith(p.replace(WILDCARD_PATH_RE, ""))
);
if (!isNested) {
continue;
}
}
for (const [index, handler] of handlers.entries()) {
// Skip middleware
if (!handler.route || handler.middleware) {
continue;
}
// We will correctly register this rule as a cached route anyway
if (handler.route === path) {
break;
}
// We are looking for handlers that will render a route _despite_ not
// having an identical path to it
if (!WILDCARD_PATH_RE.test(handler.route)) {
continue;
}
if (!path.startsWith(handler.route.replace(WILDCARD_PATH_RE, ""))) {
continue;
}
handlers.splice(index, 0, {
...handler,
route: path,
});
break;
}
}
}
1 change: 1 addition & 0 deletions test/fixture/routes/rules/[...slug].ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default defineEventHandler((event) => event.path);
4 changes: 4 additions & 0 deletions test/presets/netlify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ describe("nitro:preset:netlify", async () => {
/rules/redirect/obj https://nitro.unjs.io/ 301
/rules/nested/* /base 302
/rules/redirect /base 302
/rules/_/cached/noncached /.netlify/functions/server 200
/rules/_/noncached/cached /.netlify/builders/server 200
/rules/_/cached/* /.netlify/builders/server 200
/rules/_/noncached/* /.netlify/functions/server 200
/rules/swr-ttl/* /.netlify/builders/server 200
/rules/swr/* /.netlify/builders/server 200
/rules/static /.netlify/builders/server 200
Expand Down
15 changes: 14 additions & 1 deletion test/presets/node.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { resolve } from "pathe";
import { describe } from "vitest";
import { describe, it, expect } from "vitest";
import { startServer, setupTest, testNitro } from "../tests";

describe("nitro:preset:node", async () => {
Expand All @@ -12,4 +12,17 @@ describe("nitro:preset:node", async () => {
return res;
};
});
it("should handle nested cached route rules", async () => {
const cached = await ctx.fetch("/rules/_/noncached/cached");
expect(cached.headers.get("etag")).toBeDefined();

const noncached = await ctx.fetch("/rules/_/noncached/noncached");
expect(noncached.headers.get("etag")).toBeNull();

const cached2 = await ctx.fetch("/rules/_/cached/cached");
expect(cached2.headers.get("etag")).toBeDefined();

const noncached2 = await ctx.fetch("/rules/_/cached/noncached");
expect(noncached2.headers.get("etag")).toBeNull();
});
});
12 changes: 12 additions & 0 deletions test/presets/vercel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ describe("nitro:preset:vercel", async () => {
{
"handle": "filesystem",
},
{
"dest": "/__nitro",
"src": "/rules/_/cached/noncached",
},
{
"dest": "/__nitro",
"src": "(?<url>/rules/_/noncached/.*)",
},
{
"dest": "/__nitro--rules---cached?url=$url",
"src": "(?<url>/rules/_/cached/.*)",
},
{
"dest": "/__nitro",
"src": "/rules/dynamic",
Expand Down
4 changes: 4 additions & 0 deletions test/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ export async function setupTest(preset: string) {
},
"/rules/nested/**": { redirect: "/base", headers: { "x-test": "test" } },
"/rules/nested/override": { redirect: { to: "/other" } },
"/rules/_/noncached/cached": { swr: true },
"/rules/_/noncached/**": { swr: false, cache: false },
"/rules/_/cached/noncached": { cache: false, swr: false },
"/rules/_/cached/**": { swr: true },
},
timing: preset !== "cloudflare" && preset !== "vercel-edge",
}));
Expand Down