From ce72ea3fd1ae59cf722a9868d8e9cc99eb6b0aa1 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Wed, 30 Oct 2024 12:20:46 -0400 Subject: [PATCH 1/8] perf(netlify,netlify-edge): exclude static paths from SSR function The netlify preset was already using `preferStatic: true` which lets existing static assets take precedence over the function, but this now also avoids a function invocation for static paths that don't exist, i.e. this avoids an unnecessary dynamic 404 that could be served directly from the CDN. The `netlify-edge` preset wasn't excluding anything, so this addresses both the 404 case and the existing asset case. The 404 case is important because browsers frequently attempt to request hashed assets from previous deploys that have been invalidated. There's no reason for this to go through functions, since we know that the whole parent path is static. --- package.json | 1 + pnpm-lock.yaml | 8 + src/presets/netlify/preset.ts | 2 + src/presets/netlify/runtime/netlify-edge.ts | 3 +- src/presets/netlify/utils.ts | 8 + test/presets/netlify.test.ts | 194 +++++++++++++++++++- test/tests.ts | 1 + 7 files changed, 212 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 21eff76409..451fb5c44d 100644 --- a/package.json +++ b/package.json @@ -173,6 +173,7 @@ "@azure/static-web-apps-cli": "^1.1.10", "@cloudflare/workers-types": "^4.20241018.0", "@deno/types": "^0.0.1", + "@netlify/edge-functions": "^2.11.0", "@scalar/api-reference": "^1.25.46", "@types/archiver": "^6.0.2", "@types/aws-lambda": "^8.10.145", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2c345b4db4..26099eb6cd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -232,6 +232,9 @@ importers: '@deno/types': specifier: ^0.0.1 version: 0.0.1 + '@netlify/edge-functions': + specifier: ^2.11.0 + version: 2.11.0 '@scalar/api-reference': specifier: ^1.25.46 version: 1.25.46(@hyperjump/browser@1.1.6)(tailwindcss@3.4.13)(typescript@5.6.3) @@ -1192,6 +1195,9 @@ packages: resolution: {integrity: sha512-Yhlar6v9WQgUp/He7BdgzOz8lqMQ8sU+jkCq7Wx8Myc5YFJLbEe7lgui/V7G1qB1DJykHSGwreceSaD60Y0PUQ==} hasBin: true + '@netlify/edge-functions@2.11.0': + resolution: {integrity: sha512-DZrDHdPX44Cj7T9geKdiIwHB6OScL7QFL10voC8TApgEwY/NhKfABBGF2cbUIfbAh3IAMBeikelT8PU0MqYnyg==} + '@netlify/functions@2.8.2': resolution: {integrity: sha512-DeoAQh8LuNPvBE4qsKlezjKj0PyXDryOFJfJKo3Z1qZLKzQ21sT314KQKPVjfvw6knqijj+IO+0kHXy/TJiqNA==} engines: {node: '>=14.0.0'} @@ -6921,6 +6927,8 @@ snapshots: - encoding - supports-color + '@netlify/edge-functions@2.11.0': {} + '@netlify/functions@2.8.2': dependencies: '@netlify/serverless-functions-api': 1.26.1 diff --git a/src/presets/netlify/preset.ts b/src/presets/netlify/preset.ts index be7379fb87..d448e63902 100644 --- a/src/presets/netlify/preset.ts +++ b/src/presets/netlify/preset.ts @@ -6,6 +6,7 @@ import netlifyLegacyPresets from "./legacy/preset"; import { generateNetlifyFunction, getGeneratorString, + getStaticPaths, writeHeaders, writeRedirects, } from "./utils"; @@ -85,6 +86,7 @@ const netlifyEdge = defineNitroPreset( functions: [ { path: "/*", + excludedPath: getStaticPaths(nitro), name: "edge server handler", function: "server", generator: getGeneratorString(nitro), diff --git a/src/presets/netlify/runtime/netlify-edge.ts b/src/presets/netlify/runtime/netlify-edge.ts index 7d64738b20..09d85b2c8a 100644 --- a/src/presets/netlify/runtime/netlify-edge.ts +++ b/src/presets/netlify/runtime/netlify-edge.ts @@ -1,11 +1,12 @@ import "#nitro-internal-pollyfills"; import { useNitroApp } from "nitropack/runtime"; import { isPublicAssetURL } from "#nitro-internal-virtual/public-assets"; +import type { Context } from "@netlify/edge-functions"; const nitroApp = useNitroApp(); // https://docs.netlify.com/edge-functions/api/ -export default async function netlifyEdge(request: Request, _context: any) { +export default async function netlifyEdge(request: Request, _context: Context) { const url = new URL(request.url); if (isPublicAssetURL(url.pathname)) { diff --git a/src/presets/netlify/utils.ts b/src/presets/netlify/utils.ts index b5c9d3ba8e..74e2fefd89 100644 --- a/src/presets/netlify/utils.ts +++ b/src/presets/netlify/utils.ts @@ -92,6 +92,13 @@ export async function writeHeaders(nitro: Nitro) { await fsp.writeFile(headersPath, contents); } +export function getStaticPaths(nitro: Nitro): string[] { + const publicAssets = nitro.options.publicAssets.filter( + (dir) => dir.fallthrough !== true && dir.baseURL && dir.baseURL !== "/" + ); + return ["/.netlify/*", ...publicAssets.map((dir) => `${dir.baseURL}/*`)]; +} + // This is written to the functions directory. It just re-exports the compiled handler, // along with its config. We do this instead of compiling the entrypoint directly because // the Netlify platform actually statically analyzes the function file to read the config; @@ -103,6 +110,7 @@ export const config = { name: "server handler", generator: "${getGeneratorString(nitro)}", path: "/*", + excludedPath: ${JSON.stringify(getStaticPaths(nitro))}, preferStatic: true, }; `.trim(); diff --git a/test/presets/netlify.test.ts b/test/presets/netlify.test.ts index 900eb5d023..ff60451a03 100644 --- a/test/presets/netlify.test.ts +++ b/test/presets/netlify.test.ts @@ -1,14 +1,44 @@ import { promises as fsp } from "node:fs"; -import type { Context } from "@netlify/functions"; +import type { Context as FunctionContext } from "@netlify/functions"; +import type { Context as EdgeFunctionContext } from "@netlify/edge-functions"; import { resolve } from "pathe"; import { describe, expect, it } from "vitest"; import { getPresetTmpDir, setupTest, testNitro } from "../tests"; describe("nitro:preset:netlify", async () => { + const publicDir = resolve(getPresetTmpDir("netlify"), "dist"); const ctx = await setupTest("netlify", { config: { + framework: { + name: "mock-framework", + version: "1.2.3", + }, + publicAssets: [ + { + fallthrough: true, + baseURL: "foo", + dir: publicDir, + }, + { + fallthrough: false, + dir: publicDir, + }, + { + fallthrough: true, + dir: publicDir, + }, + { + baseURL: "icons", + dir: publicDir, + }, + { + fallthrough: false, + baseURL: "nested/fonts", + dir: publicDir, + }, + ], output: { - publicDir: resolve(getPresetTmpDir("netlify"), "dist"), + publicDir, }, netlify: { images: { @@ -22,7 +52,9 @@ describe("nitro:preset:netlify", async () => { async () => { const { default: handler } = (await import( resolve(ctx.outDir, "server/main.mjs") - )) as { default: (req: Request, ctx: Context) => Promise }; + )) as { + default: (req: Request, _ctx: FunctionContext) => Promise; + }; return async ({ url: rawRelativeUrl, headers, method, body }) => { // creating new URL object to parse query easier const url = new URL(`https://example.com${rawRelativeUrl}`); @@ -31,7 +63,7 @@ describe("nitro:preset:netlify", async () => { method, body, }); - const res = await handler(req, {} as Context); + const res = await handler(req, {} as FunctionContext); return res; }; }, @@ -86,6 +118,24 @@ describe("nitro:preset:netlify", async () => { } `); }); + it("writes server/server.mjs with static paths excluded", async () => { + const serverFunctionFile = await fsp.readFile( + resolve(ctx.outDir, "server/server.mjs"), + "utf8" + ); + expect(serverFunctionFile).toEqual( + ` +export { default } from "./main.mjs"; +export const config = { + name: "server handler", + generator: "mock-framework@1.2.3", + path: "/*", + excludedPath: ["/.netlify/*","/icons/*","/nested/fonts/*","/build/*"], + preferStatic: true, +}; + `.trim() + ); + }); describe("matching ISR route rule with no max-age", () => { it("sets Netlify-CDN-Cache-Control header with revalidation after 1 year and durable directive", async () => { const { headers } = await callHandler({ url: "/rules/isr" }); @@ -137,3 +187,139 @@ describe("nitro:preset:netlify", async () => { } ); }); + +describe("nitro:preset:netlify-edge", async () => { + const publicDir = resolve(getPresetTmpDir("netlify-edge"), "dist"); + const ctx = await setupTest("netlify-edge", { + config: { + framework: { + name: "mock-framework", + version: "1.2.3", + }, + publicAssets: [ + { + fallthrough: true, + baseURL: "foo", + dir: publicDir, + }, + { + fallthrough: false, + dir: publicDir, + }, + { + fallthrough: true, + dir: publicDir, + }, + { + baseURL: "icons", + dir: publicDir, + }, + { + fallthrough: false, + baseURL: "nested/fonts", + dir: publicDir, + }, + ], + output: { + publicDir, + }, + netlify: { + images: { + remote_images: ["https://example.com/.*"], + }, + }, + }, + }); + testNitro( + ctx, + async () => { + const { default: handler } = (await import( + resolve(ctx.rootDir, ".netlify/edge-functions/server/server.js") + )) as { + default: (req: Request, _ctx: EdgeFunctionContext) => Promise; + }; + return async ({ url: rawRelativeUrl, headers, method, body }) => { + // creating new URL object to parse query easier + const url = new URL(`https://example.com${rawRelativeUrl}`); + const req = new Request(url, { + headers: headers ?? {}, + method, + body, + }); + const res = await handler(req, {} as EdgeFunctionContext); + if (!(res instanceof Response)) + // The Netlify Edge Function handler API allows returning `undefined` but this + // test helper only supports a Response or this shape. + return { + data: undefined, + status: 404, + headers: {}, + }; + return res; + }; + }, + () => { + it("adds route rules - redirects", async () => { + const redirects = await fsp.readFile( + resolve(ctx.outDir, "../dist/_redirects"), + "utf8" + ); + + expect(redirects).toMatchInlineSnapshot(` + "/rules/nested/override /other 302 + /rules/redirect/wildcard/* https://nitro.unjs.io/:splat 302 + /rules/redirect/obj https://nitro.unjs.io/ 301 + /rules/nested/* /base 302 + /rules/redirect /base 302 + " + `); + }); + it("adds route rules - headers", async () => { + const headers = await fsp.readFile( + resolve(ctx.outDir, "../dist/_headers"), + "utf8" + ); + + expect(headers).toMatchInlineSnapshot(` + "/rules/headers + cache-control: s-maxage=60 + /rules/cors + access-control-allow-origin: * + access-control-allow-methods: GET + access-control-allow-headers: * + access-control-max-age: 0 + /rules/nested/* + x-test: test + /build/* + cache-control: public, max-age=3600, immutable + " + `); + }); + it("writes edge-functions/manifest.json with static paths excluded", async () => { + const manifestFile = JSON.parse( + await fsp.readFile( + resolve(ctx.rootDir, ".netlify/edge-functions/manifest.json"), + "utf8" + ) + ); + expect(manifestFile).toEqual({ + version: 1, + functions: [ + { + path: "/*", + excludedPath: [ + "/.netlify/*", + "/icons/*", + "/nested/fonts/*", + "/build/*", + ], + name: "edge server handler", + function: "server", + generator: "mock-framework@1.2.3", + }, + ], + }); + }); + } + ); +}); diff --git a/test/tests.ts b/test/tests.ts index ccb94269df..091c7818ff 100644 --- a/test/tests.ts +++ b/test/tests.ts @@ -88,6 +88,7 @@ export async function setupTest( "cloudflare-module", "cloudflare-module-legacy", "cloudflare-pages", + "netlify-edge", "vercel-edge", "winterjs", ].includes(preset), From e24ca007cc767ad0acb2f020684a1775a67f17d9 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Thu, 31 Oct 2024 07:32:02 -0400 Subject: [PATCH 2/8] test(presets): centralize framework, publicAssets fixture --- test/fixture/nitro.config.ts | 22 ++++++++ test/presets/cloudflare-pages.test.ts | 2 + test/presets/netlify.test.ts | 76 ++++++--------------------- test/presets/nitro-dev.test.ts | 9 +++- test/presets/vercel.test.ts | 14 +++++ 5 files changed, 61 insertions(+), 62 deletions(-) diff --git a/test/fixture/nitro.config.ts b/test/fixture/nitro.config.ts index 097b9f27bf..380ec5ce31 100644 --- a/test/fixture/nitro.config.ts +++ b/test/fixture/nitro.config.ts @@ -4,6 +4,10 @@ import { defineNitroConfig } from "nitropack/config"; export default defineNitroConfig({ compressPublicAssets: true, compatibilityDate: "2024-09-19", + framework: { + name: "mock-framework", + version: "1.2.3", + }, imports: { presets: [ { @@ -57,6 +61,24 @@ export default defineNitroConfig({ dir: "public/build", maxAge: 3600, }, + { + fallthrough: true, + baseURL: "with-fallthrough", + dir: "with-fallthrough-dir", + }, + { + fallthrough: true, + dir: "with-fallthrough-no-baseURL-dir", + }, + { + baseURL: "with-default-fallthrough", + dir: "with-default-fallthrough-dir", + }, + { + fallthrough: false, + baseURL: "nested/no-fallthrough", + dir: "nested/no-fallthrough-dir", + }, ], tasks: { "db:migrate": { description: "Migrate database" }, diff --git a/test/presets/cloudflare-pages.test.ts b/test/presets/cloudflare-pages.test.ts index f6bbf25f9b..ee34f92a0c 100644 --- a/test/presets/cloudflare-pages.test.ts +++ b/test/presets/cloudflare-pages.test.ts @@ -41,6 +41,8 @@ describe.skipIf(isWindows)("nitro:preset:cloudflare-pages", async () => { "/blog/static/*", "/cf-pages-exclude/*", "/build/*", + "/with-default-fallthrough/*", + "/nested/no-fallthrough/*", "/_openapi.json", "/_openapi.json.br", "/_openapi.json.gz", diff --git a/test/presets/netlify.test.ts b/test/presets/netlify.test.ts index ff60451a03..b97c56d667 100644 --- a/test/presets/netlify.test.ts +++ b/test/presets/netlify.test.ts @@ -6,39 +6,10 @@ import { describe, expect, it } from "vitest"; import { getPresetTmpDir, setupTest, testNitro } from "../tests"; describe("nitro:preset:netlify", async () => { - const publicDir = resolve(getPresetTmpDir("netlify"), "dist"); const ctx = await setupTest("netlify", { config: { - framework: { - name: "mock-framework", - version: "1.2.3", - }, - publicAssets: [ - { - fallthrough: true, - baseURL: "foo", - dir: publicDir, - }, - { - fallthrough: false, - dir: publicDir, - }, - { - fallthrough: true, - dir: publicDir, - }, - { - baseURL: "icons", - dir: publicDir, - }, - { - fallthrough: false, - baseURL: "nested/fonts", - dir: publicDir, - }, - ], output: { - publicDir, + publicDir: resolve(getPresetTmpDir("netlify"), "dist"), }, netlify: { images: { @@ -83,6 +54,7 @@ describe("nitro:preset:netlify", async () => { " `); }); + it("adds route rules - headers", async () => { const headers = await fsp.readFile( resolve(ctx.outDir, "../dist/_headers"), @@ -104,6 +76,7 @@ describe("nitro:preset:netlify", async () => { " `); }); + it("writes config.json", async () => { const config = await fsp .readFile(resolve(ctx.outDir, "../deploy/v1/config.json"), "utf8") @@ -118,6 +91,7 @@ describe("nitro:preset:netlify", async () => { } `); }); + it("writes server/server.mjs with static paths excluded", async () => { const serverFunctionFile = await fsp.readFile( resolve(ctx.outDir, "server/server.mjs"), @@ -130,12 +104,13 @@ export const config = { name: "server handler", generator: "mock-framework@1.2.3", path: "/*", - excludedPath: ["/.netlify/*","/icons/*","/nested/fonts/*","/build/*"], + excludedPath: ["/.netlify/*","/build/*","/with-default-fallthrough/*","/nested/no-fallthrough/*"], preferStatic: true, }; `.trim() ); }); + describe("matching ISR route rule with no max-age", () => { it("sets Netlify-CDN-Cache-Control header with revalidation after 1 year and durable directive", async () => { const { headers } = await callHandler({ url: "/rules/isr" }); @@ -143,6 +118,7 @@ export const config = { (headers as Record)["netlify-cdn-cache-control"] ).toBe("public, max-age=31536000, must-revalidate, durable"); }); + it("sets Cache-Control header with immediate revalidation", async () => { const { headers } = await callHandler({ url: "/rules/isr" }); expect((headers as Record)["cache-control"]).toBe( @@ -150,6 +126,7 @@ export const config = { ); }); }); + describe("matching ISR route rule with a max-age", () => { it("sets Netlify-CDN-Cache-Control header with SWC=1yr, given max-age, and durable directive", async () => { const { headers } = await callHandler({ url: "/rules/isr-ttl" }); @@ -159,6 +136,7 @@ export const config = { "public, max-age=60, stale-while-revalidate=31536000, durable" ); }); + it("sets Cache-Control header with immediate revalidation", async () => { const { headers } = await callHandler({ url: "/rules/isr-ttl" }); expect((headers as Record)["cache-control"]).toBe( @@ -166,6 +144,7 @@ export const config = { ); }); }); + it("does not overwrite Cache-Control headers given a matching non-ISR route rule", async () => { const { headers } = await callHandler({ url: "/rules/dynamic" }); expect( @@ -175,6 +154,7 @@ export const config = { (headers as Record)["netlify-cdn-cache-control"] ).not.toBeDefined(); }); + // Regression test for https://github.com/unjs/nitro/issues/2431 it("matches paths with a query string", async () => { const { headers } = await callHandler({ @@ -192,34 +172,6 @@ describe("nitro:preset:netlify-edge", async () => { const publicDir = resolve(getPresetTmpDir("netlify-edge"), "dist"); const ctx = await setupTest("netlify-edge", { config: { - framework: { - name: "mock-framework", - version: "1.2.3", - }, - publicAssets: [ - { - fallthrough: true, - baseURL: "foo", - dir: publicDir, - }, - { - fallthrough: false, - dir: publicDir, - }, - { - fallthrough: true, - dir: publicDir, - }, - { - baseURL: "icons", - dir: publicDir, - }, - { - fallthrough: false, - baseURL: "nested/fonts", - dir: publicDir, - }, - ], output: { publicDir, }, @@ -274,6 +226,7 @@ describe("nitro:preset:netlify-edge", async () => { " `); }); + it("adds route rules - headers", async () => { const headers = await fsp.readFile( resolve(ctx.outDir, "../dist/_headers"), @@ -295,6 +248,7 @@ describe("nitro:preset:netlify-edge", async () => { " `); }); + it("writes edge-functions/manifest.json with static paths excluded", async () => { const manifestFile = JSON.parse( await fsp.readFile( @@ -309,9 +263,9 @@ describe("nitro:preset:netlify-edge", async () => { path: "/*", excludedPath: [ "/.netlify/*", - "/icons/*", - "/nested/fonts/*", "/build/*", + "/with-default-fallthrough/*", + "/nested/no-fallthrough/*", ], name: "edge server handler", function: "server", diff --git a/test/presets/nitro-dev.test.ts b/test/presets/nitro-dev.test.ts index fb58b99510..e5b9999fcb 100644 --- a/test/presets/nitro-dev.test.ts +++ b/test/presets/nitro-dev.test.ts @@ -4,7 +4,14 @@ import { describe, expect, it } from "vitest"; import { setupTest, testNitro } from "../tests"; describe.skipIf(isCI)("nitro:preset:nitro-dev", async () => { - const ctx = await setupTest("nitro-dev"); + const ctx = await setupTest("nitro-dev", { + config: { + framework: { + name: "nitro", + version: "", + }, + }, + }); testNitro( ctx, () => { diff --git a/test/presets/vercel.test.ts b/test/presets/vercel.test.ts index e88d814d19..d25bcd3308 100644 --- a/test/presets/vercel.test.ts +++ b/test/presets/vercel.test.ts @@ -103,6 +103,20 @@ describe("nitro:preset:vercel", async () => { }, "src": "/build(.*)", }, + { + "continue": true, + "headers": { + "cache-control": "public,max-age=31536000,immutable", + }, + "src": "/with-default-fallthrough(.*)", + }, + { + "continue": true, + "headers": { + "cache-control": "public,max-age=31536000,immutable", + }, + "src": "/nested/no-fallthrough(.*)", + }, { "handle": "filesystem", }, From b012b2db784a858322f0d50d7bf1c4e5bfb67f7f Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Thu, 31 Oct 2024 07:51:48 -0400 Subject: [PATCH 3/8] test(netlify-edge): fix prerendered route test --- test/presets/netlify.test.ts | 2 +- test/tests.ts | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/presets/netlify.test.ts b/test/presets/netlify.test.ts index b97c56d667..3f629dad57 100644 --- a/test/presets/netlify.test.ts +++ b/test/presets/netlify.test.ts @@ -201,7 +201,7 @@ describe("nitro:preset:netlify-edge", async () => { const res = await handler(req, {} as EdgeFunctionContext); if (!(res instanceof Response)) // The Netlify Edge Function handler API allows returning `undefined` but this - // test helper only supports a Response or this shape. + // test helper only supports a Response or this shape. This is equivalent to a 404. return { data: undefined, status: 404, diff --git a/test/tests.ts b/test/tests.ts index 091c7818ff..3118c0cade 100644 --- a/test/tests.ts +++ b/test/tests.ts @@ -224,16 +224,22 @@ export function testNitro( _handler = await getHandler(); }, 25_000); - it("API Works", async () => { - const { data: helloData } = await callHandler({ url: "/api/hello" }); - expect(helloData).to.toMatchObject({ message: "Hello API" }); + // netlify-edge intentionally ignores prerendered routes + it.skipIf(ctx.preset === "netlify-edge")( + "prerendered API routes work", + async () => { + const { data: helloData } = await callHandler({ url: "/api/hello" }); + expect(helloData).to.toMatchObject({ message: "Hello API" }); - if (ctx.nitro?.options.serveStatic) { - // /api/hey is expected to be prerendered - const { data: heyData } = await callHandler({ url: "/api/hey" }); - expect(heyData).to.have.string("Hey API"); + if (ctx.nitro?.options.serveStatic) { + // /api/hey is expected to be prerendered + const { data: heyData } = await callHandler({ url: "/api/hey" }); + expect(heyData).to.have.string("Hey API"); + } } + ); + it("API Works", async () => { const { data: kebabData } = await callHandler({ url: "/api/kebab" }); expect(kebabData).to.have.string("hello-world"); From b7a3f01777cf4b2dc4b037973c69beed83d76ad0 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Thu, 31 Oct 2024 10:37:33 -0400 Subject: [PATCH 4/8] refactor(netlify): pass publicAssets to getStaticPaths and unit test it also, ensure the right URL format on paths --- src/presets/netlify/preset.ts | 2 +- src/presets/netlify/utils.ts | 20 +++++++---- test/fixture/nitro.config.ts | 18 ---------- test/presets/cloudflare-pages.test.ts | 2 -- test/presets/netlify.test.ts | 21 +++++++---- test/presets/vercel.test.ts | 14 -------- test/unit/netlify.utils.test.ts | 51 +++++++++++++++++++++++++++ 7 files changed, 79 insertions(+), 49 deletions(-) create mode 100644 test/unit/netlify.utils.test.ts diff --git a/src/presets/netlify/preset.ts b/src/presets/netlify/preset.ts index d448e63902..1ede327c68 100644 --- a/src/presets/netlify/preset.ts +++ b/src/presets/netlify/preset.ts @@ -86,7 +86,7 @@ const netlifyEdge = defineNitroPreset( functions: [ { path: "/*", - excludedPath: getStaticPaths(nitro), + excludedPath: getStaticPaths(nitro.options.publicAssets), name: "edge server handler", function: "server", generator: getGeneratorString(nitro), diff --git a/src/presets/netlify/utils.ts b/src/presets/netlify/utils.ts index 74e2fefd89..e1a7bd6c22 100644 --- a/src/presets/netlify/utils.ts +++ b/src/presets/netlify/utils.ts @@ -1,6 +1,7 @@ import { existsSync, promises as fsp } from "node:fs"; -import type { Nitro } from "nitropack/types"; +import type { Nitro, PublicAssetDir } from "nitropack/types"; import { join } from "pathe"; +import { joinURL } from "ufo"; export async function writeRedirects(nitro: Nitro) { const redirectsPath = join(nitro.options.output.publicDir, "_redirects"); @@ -92,11 +93,16 @@ export async function writeHeaders(nitro: Nitro) { await fsp.writeFile(headersPath, contents); } -export function getStaticPaths(nitro: Nitro): string[] { - const publicAssets = nitro.options.publicAssets.filter( - (dir) => dir.fallthrough !== true && dir.baseURL && dir.baseURL !== "/" - ); - return ["/.netlify/*", ...publicAssets.map((dir) => `${dir.baseURL}/*`)]; +export function getStaticPaths(publicAssets: PublicAssetDir[]): string[] { + return [ + "/.netlify", + ...publicAssets + .filter( + (path) => + path.fallthrough !== true && path.baseURL && path.baseURL !== "/" + ) + .map(({ baseURL }) => baseURL), + ].map((url) => joinURL("/", url!, "*")); } // This is written to the functions directory. It just re-exports the compiled handler, @@ -110,7 +116,7 @@ export const config = { name: "server handler", generator: "${getGeneratorString(nitro)}", path: "/*", - excludedPath: ${JSON.stringify(getStaticPaths(nitro))}, + excludedPath: ${JSON.stringify(getStaticPaths(nitro.options.publicAssets))}, preferStatic: true, }; `.trim(); diff --git a/test/fixture/nitro.config.ts b/test/fixture/nitro.config.ts index 380ec5ce31..18dc15c6cf 100644 --- a/test/fixture/nitro.config.ts +++ b/test/fixture/nitro.config.ts @@ -61,24 +61,6 @@ export default defineNitroConfig({ dir: "public/build", maxAge: 3600, }, - { - fallthrough: true, - baseURL: "with-fallthrough", - dir: "with-fallthrough-dir", - }, - { - fallthrough: true, - dir: "with-fallthrough-no-baseURL-dir", - }, - { - baseURL: "with-default-fallthrough", - dir: "with-default-fallthrough-dir", - }, - { - fallthrough: false, - baseURL: "nested/no-fallthrough", - dir: "nested/no-fallthrough-dir", - }, ], tasks: { "db:migrate": { description: "Migrate database" }, diff --git a/test/presets/cloudflare-pages.test.ts b/test/presets/cloudflare-pages.test.ts index ee34f92a0c..f6bbf25f9b 100644 --- a/test/presets/cloudflare-pages.test.ts +++ b/test/presets/cloudflare-pages.test.ts @@ -41,8 +41,6 @@ describe.skipIf(isWindows)("nitro:preset:cloudflare-pages", async () => { "/blog/static/*", "/cf-pages-exclude/*", "/build/*", - "/with-default-fallthrough/*", - "/nested/no-fallthrough/*", "/_openapi.json", "/_openapi.json.br", "/_openapi.json.gz", diff --git a/test/presets/netlify.test.ts b/test/presets/netlify.test.ts index 3f629dad57..33c777a115 100644 --- a/test/presets/netlify.test.ts +++ b/test/presets/netlify.test.ts @@ -8,6 +8,12 @@ import { getPresetTmpDir, setupTest, testNitro } from "../tests"; describe("nitro:preset:netlify", async () => { const ctx = await setupTest("netlify", { config: { + publicAssets: [ + { + dir: "dist/_nuxt", + baseURL: "_nuxt", + }, + ], output: { publicDir: resolve(getPresetTmpDir("netlify"), "dist"), }, @@ -104,7 +110,7 @@ export const config = { name: "server handler", generator: "mock-framework@1.2.3", path: "/*", - excludedPath: ["/.netlify/*","/build/*","/with-default-fallthrough/*","/nested/no-fallthrough/*"], + excludedPath: ["/.netlify/*","/_nuxt/*","/build/*"], preferStatic: true, }; `.trim() @@ -172,6 +178,12 @@ describe("nitro:preset:netlify-edge", async () => { const publicDir = resolve(getPresetTmpDir("netlify-edge"), "dist"); const ctx = await setupTest("netlify-edge", { config: { + publicAssets: [ + { + dir: "dist/_nuxt", + baseURL: "_nuxt", + }, + ], output: { publicDir, }, @@ -261,12 +273,7 @@ describe("nitro:preset:netlify-edge", async () => { functions: [ { path: "/*", - excludedPath: [ - "/.netlify/*", - "/build/*", - "/with-default-fallthrough/*", - "/nested/no-fallthrough/*", - ], + excludedPath: ["/.netlify/*", "/_nuxt/*", "/build/*"], name: "edge server handler", function: "server", generator: "mock-framework@1.2.3", diff --git a/test/presets/vercel.test.ts b/test/presets/vercel.test.ts index d25bcd3308..e88d814d19 100644 --- a/test/presets/vercel.test.ts +++ b/test/presets/vercel.test.ts @@ -103,20 +103,6 @@ describe("nitro:preset:vercel", async () => { }, "src": "/build(.*)", }, - { - "continue": true, - "headers": { - "cache-control": "public,max-age=31536000,immutable", - }, - "src": "/with-default-fallthrough(.*)", - }, - { - "continue": true, - "headers": { - "cache-control": "public,max-age=31536000,immutable", - }, - "src": "/nested/no-fallthrough(.*)", - }, { "handle": "filesystem", }, diff --git a/test/unit/netlify.utils.test.ts b/test/unit/netlify.utils.test.ts new file mode 100644 index 0000000000..8d562f02f9 --- /dev/null +++ b/test/unit/netlify.utils.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; +import { getStaticPaths } from "../../src/presets/netlify/utils"; + +describe("getStaticPaths", () => { + it("always returns `/.netlify/*`", () => { + expect(getStaticPaths([])).toEqual(["/.netlify/*"]); + }); + + it("returns a pattern with a leading slash for each non-fallthrough non-root public asset path", () => { + const publicAssets = [ + { + fallthrough: true, + baseURL: "with-fallthrough", + dir: "with-fallthrough-dir", + maxAge: 0, + }, + { + fallthrough: true, + dir: "with-fallthrough-no-baseURL-dir", + maxAge: 0, + }, + { + fallthrough: false, + dir: "no-fallthrough-no-baseURL-dir", + maxAge: 0, + }, + { + fallthrough: false, + dir: "no-fallthrough-root-baseURL-dir", + baseURL: "/", + maxAge: 0, + }, + { + baseURL: "with-default-fallthrough", + dir: "with-default-fallthrough-dir", + maxAge: 0, + }, + { + fallthrough: false, + baseURL: "nested/no-fallthrough", + dir: "nested/no-fallthrough-dir", + maxAge: 0, + }, + ]; + expect(getStaticPaths(publicAssets)).toEqual([ + "/.netlify/*", + "/with-default-fallthrough/*", + "/nested/no-fallthrough/*", + ]); + }); +}); From 288b7a4232792ecf74e6887a509f272f2b29d8a3 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Thu, 31 Oct 2024 10:40:12 -0400 Subject: [PATCH 5/8] test(netlify-edge): split new netlify-edge tests out into new PR --- test/presets/netlify.test.ts | 112 ----------------------------------- 1 file changed, 112 deletions(-) diff --git a/test/presets/netlify.test.ts b/test/presets/netlify.test.ts index 33c777a115..19bb0545b5 100644 --- a/test/presets/netlify.test.ts +++ b/test/presets/netlify.test.ts @@ -1,6 +1,5 @@ import { promises as fsp } from "node:fs"; import type { Context as FunctionContext } from "@netlify/functions"; -import type { Context as EdgeFunctionContext } from "@netlify/edge-functions"; import { resolve } from "pathe"; import { describe, expect, it } from "vitest"; import { getPresetTmpDir, setupTest, testNitro } from "../tests"; @@ -173,114 +172,3 @@ export const config = { } ); }); - -describe("nitro:preset:netlify-edge", async () => { - const publicDir = resolve(getPresetTmpDir("netlify-edge"), "dist"); - const ctx = await setupTest("netlify-edge", { - config: { - publicAssets: [ - { - dir: "dist/_nuxt", - baseURL: "_nuxt", - }, - ], - output: { - publicDir, - }, - netlify: { - images: { - remote_images: ["https://example.com/.*"], - }, - }, - }, - }); - testNitro( - ctx, - async () => { - const { default: handler } = (await import( - resolve(ctx.rootDir, ".netlify/edge-functions/server/server.js") - )) as { - default: (req: Request, _ctx: EdgeFunctionContext) => Promise; - }; - return async ({ url: rawRelativeUrl, headers, method, body }) => { - // creating new URL object to parse query easier - const url = new URL(`https://example.com${rawRelativeUrl}`); - const req = new Request(url, { - headers: headers ?? {}, - method, - body, - }); - const res = await handler(req, {} as EdgeFunctionContext); - if (!(res instanceof Response)) - // The Netlify Edge Function handler API allows returning `undefined` but this - // test helper only supports a Response or this shape. This is equivalent to a 404. - return { - data: undefined, - status: 404, - headers: {}, - }; - return res; - }; - }, - () => { - it("adds route rules - redirects", async () => { - const redirects = await fsp.readFile( - resolve(ctx.outDir, "../dist/_redirects"), - "utf8" - ); - - expect(redirects).toMatchInlineSnapshot(` - "/rules/nested/override /other 302 - /rules/redirect/wildcard/* https://nitro.unjs.io/:splat 302 - /rules/redirect/obj https://nitro.unjs.io/ 301 - /rules/nested/* /base 302 - /rules/redirect /base 302 - " - `); - }); - - it("adds route rules - headers", async () => { - const headers = await fsp.readFile( - resolve(ctx.outDir, "../dist/_headers"), - "utf8" - ); - - expect(headers).toMatchInlineSnapshot(` - "/rules/headers - cache-control: s-maxage=60 - /rules/cors - access-control-allow-origin: * - access-control-allow-methods: GET - access-control-allow-headers: * - access-control-max-age: 0 - /rules/nested/* - x-test: test - /build/* - cache-control: public, max-age=3600, immutable - " - `); - }); - - it("writes edge-functions/manifest.json with static paths excluded", async () => { - const manifestFile = JSON.parse( - await fsp.readFile( - resolve(ctx.rootDir, ".netlify/edge-functions/manifest.json"), - "utf8" - ) - ); - expect(manifestFile).toEqual({ - version: 1, - functions: [ - { - path: "/*", - excludedPath: ["/.netlify/*", "/_nuxt/*", "/build/*"], - name: "edge server handler", - function: "server", - generator: "mock-framework@1.2.3", - }, - ], - }); - }); - } - ); -}); From af586de1c31bc710a0740ec6fd27330db9f32b79 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Thu, 31 Oct 2024 16:21:11 +0100 Subject: [PATCH 6/8] refactor: update test --- test/fixture/nitro.config.ts | 4 +- test/presets/netlify.test.ts | 78 +++++++++++++++++++++++++-------- test/unit/netlify.utils.test.ts | 51 --------------------- 3 files changed, 62 insertions(+), 71 deletions(-) delete mode 100644 test/unit/netlify.utils.test.ts diff --git a/test/fixture/nitro.config.ts b/test/fixture/nitro.config.ts index 18dc15c6cf..4d050365d0 100644 --- a/test/fixture/nitro.config.ts +++ b/test/fixture/nitro.config.ts @@ -5,8 +5,8 @@ export default defineNitroConfig({ compressPublicAssets: true, compatibilityDate: "2024-09-19", framework: { - name: "mock-framework", - version: "1.2.3", + name: "nitro", + version: "2.x", }, imports: { presets: [ diff --git a/test/presets/netlify.test.ts b/test/presets/netlify.test.ts index 19bb0545b5..80b5d592b0 100644 --- a/test/presets/netlify.test.ts +++ b/test/presets/netlify.test.ts @@ -2,17 +2,12 @@ import { promises as fsp } from "node:fs"; import type { Context as FunctionContext } from "@netlify/functions"; import { resolve } from "pathe"; import { describe, expect, it } from "vitest"; +import { getStaticPaths } from "../../src/presets/netlify/utils"; import { getPresetTmpDir, setupTest, testNitro } from "../tests"; describe("nitro:preset:netlify", async () => { const ctx = await setupTest("netlify", { config: { - publicAssets: [ - { - dir: "dist/_nuxt", - baseURL: "_nuxt", - }, - ], output: { publicDir: resolve(getPresetTmpDir("netlify"), "dist"), }, @@ -102,18 +97,16 @@ describe("nitro:preset:netlify", async () => { resolve(ctx.outDir, "server/server.mjs"), "utf8" ); - expect(serverFunctionFile).toEqual( - ` -export { default } from "./main.mjs"; -export const config = { - name: "server handler", - generator: "mock-framework@1.2.3", - path: "/*", - excludedPath: ["/.netlify/*","/_nuxt/*","/build/*"], - preferStatic: true, -}; - `.trim() - ); + expect(serverFunctionFile).toMatchInlineSnapshot(` + "export { default } from "./main.mjs"; + export const config = { + name: "server handler", + generator: "nitro@2.x", + path: "/*", + excludedPath: ["/.netlify/*","/build/*"], + preferStatic: true, + };" + `); }); describe("matching ISR route rule with no max-age", () => { @@ -171,4 +164,53 @@ export const config = { }); } ); + + describe("getStaticPaths", () => { + it("always returns `/.netlify/*`", () => { + expect(getStaticPaths([])).toEqual(["/.netlify/*"]); + }); + + it("returns a pattern with a leading slash for each non-fallthrough non-root public asset path", () => { + const publicAssets = [ + { + fallthrough: true, + baseURL: "with-fallthrough", + dir: "with-fallthrough-dir", + maxAge: 0, + }, + { + fallthrough: true, + dir: "with-fallthrough-no-baseURL-dir", + maxAge: 0, + }, + { + fallthrough: false, + dir: "no-fallthrough-no-baseURL-dir", + maxAge: 0, + }, + { + fallthrough: false, + dir: "no-fallthrough-root-baseURL-dir", + baseURL: "/", + maxAge: 0, + }, + { + baseURL: "with-default-fallthrough", + dir: "with-default-fallthrough-dir", + maxAge: 0, + }, + { + fallthrough: false, + baseURL: "nested/no-fallthrough", + dir: "nested/no-fallthrough-dir", + maxAge: 0, + }, + ]; + expect(getStaticPaths(publicAssets)).toEqual([ + "/.netlify/*", + "/with-default-fallthrough/*", + "/nested/no-fallthrough/*", + ]); + }); + }); }); diff --git a/test/unit/netlify.utils.test.ts b/test/unit/netlify.utils.test.ts deleted file mode 100644 index 8d562f02f9..0000000000 --- a/test/unit/netlify.utils.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { getStaticPaths } from "../../src/presets/netlify/utils"; - -describe("getStaticPaths", () => { - it("always returns `/.netlify/*`", () => { - expect(getStaticPaths([])).toEqual(["/.netlify/*"]); - }); - - it("returns a pattern with a leading slash for each non-fallthrough non-root public asset path", () => { - const publicAssets = [ - { - fallthrough: true, - baseURL: "with-fallthrough", - dir: "with-fallthrough-dir", - maxAge: 0, - }, - { - fallthrough: true, - dir: "with-fallthrough-no-baseURL-dir", - maxAge: 0, - }, - { - fallthrough: false, - dir: "no-fallthrough-no-baseURL-dir", - maxAge: 0, - }, - { - fallthrough: false, - dir: "no-fallthrough-root-baseURL-dir", - baseURL: "/", - maxAge: 0, - }, - { - baseURL: "with-default-fallthrough", - dir: "with-default-fallthrough-dir", - maxAge: 0, - }, - { - fallthrough: false, - baseURL: "nested/no-fallthrough", - dir: "nested/no-fallthrough-dir", - maxAge: 0, - }, - ]; - expect(getStaticPaths(publicAssets)).toEqual([ - "/.netlify/*", - "/with-default-fallthrough/*", - "/nested/no-fallthrough/*", - ]); - }); -}); From b293df7a81afdc80da5fab9b58a15b75264714f9 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Thu, 31 Oct 2024 16:22:53 +0100 Subject: [PATCH 7/8] test: revert change in nitro-dev --- test/presets/nitro-dev.test.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/presets/nitro-dev.test.ts b/test/presets/nitro-dev.test.ts index e5b9999fcb..45cf967dfa 100644 --- a/test/presets/nitro-dev.test.ts +++ b/test/presets/nitro-dev.test.ts @@ -4,14 +4,7 @@ import { describe, expect, it } from "vitest"; import { setupTest, testNitro } from "../tests"; describe.skipIf(isCI)("nitro:preset:nitro-dev", async () => { - const ctx = await setupTest("nitro-dev", { - config: { - framework: { - name: "nitro", - version: "", - }, - }, - }); + const ctx = await setupTest("nitro-dev", {}); testNitro( ctx, () => { From 6cabd167997beda81ae437ddec589ba810955d7d Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Thu, 31 Oct 2024 16:24:25 +0100 Subject: [PATCH 8/8] update test --- test/presets/nitro-dev.test.ts | 2 +- test/tests.ts | 22 ++++++++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/test/presets/nitro-dev.test.ts b/test/presets/nitro-dev.test.ts index 45cf967dfa..fb58b99510 100644 --- a/test/presets/nitro-dev.test.ts +++ b/test/presets/nitro-dev.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; import { setupTest, testNitro } from "../tests"; describe.skipIf(isCI)("nitro:preset:nitro-dev", async () => { - const ctx = await setupTest("nitro-dev", {}); + const ctx = await setupTest("nitro-dev"); testNitro( ctx, () => { diff --git a/test/tests.ts b/test/tests.ts index 3118c0cade..7839c06808 100644 --- a/test/tests.ts +++ b/test/tests.ts @@ -224,22 +224,16 @@ export function testNitro( _handler = await getHandler(); }, 25_000); - // netlify-edge intentionally ignores prerendered routes - it.skipIf(ctx.preset === "netlify-edge")( - "prerendered API routes work", - async () => { - const { data: helloData } = await callHandler({ url: "/api/hello" }); - expect(helloData).to.toMatchObject({ message: "Hello API" }); + it("API Works", async () => { + const { data: helloData } = await callHandler({ url: "/api/hello" }); + expect(helloData).to.toMatchObject({ message: "Hello API" }); - if (ctx.nitro?.options.serveStatic) { - // /api/hey is expected to be prerendered - const { data: heyData } = await callHandler({ url: "/api/hey" }); - expect(heyData).to.have.string("Hey API"); - } + if (ctx.nitro?.options.serveStatic) { + // /api/hey is expected to be prerendered + const { data: heyData } = await callHandler({ url: "/api/hey" }); + expect(heyData).to.have.string("Hey API"); } - ); - it("API Works", async () => { const { data: kebabData } = await callHandler({ url: "/api/kebab" }); expect(kebabData).to.have.string("hello-world"); @@ -565,7 +559,7 @@ export function testNitro( ); it.skipIf(ctx.isWorker || ctx.isDev)( - "public files can be un-ignored with patterns", + "public files can be un-ignored with patterns", async () => { expect((await callHandler({ url: "/_unignored.txt" })).status).toBe( 200