Skip to content

Commit

Permalink
fix: apply cached rules to overlaping wildcard patterns (#906)
Browse files Browse the repository at this point in the history
  • Loading branch information
danielroe authored Feb 7, 2023
1 parent fc0be2a commit aa5a2f4
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 7 deletions.
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

0 comments on commit aa5a2f4

Please sign in to comment.