address comments

Author: mathu97

packages/@apphosting/adapter-nextjs/src/bin/build.spec.ts

@@ -4,12 +4,14 @@ import fs from "fs";
 import yaml from "yaml";
 import path from "path";
 import os from "os";
-import { OutputBundleOptions } from "../interfaces.js";
+import { OutputBundleOptions, AdapterMetadata } from "../interfaces.js";
 
 describe("build commands", () => {
   let tmpDir: string;
   let outputBundleOptions: OutputBundleOptions;
   let defaultNextVersion: string;
+  let adapterMetadata: AdapterMetadata;
+
   beforeEach(() => {
     tmpDir = generateTmpDir();
     outputBundleOptions = {
@@ -21,10 +23,14 @@ describe("build commands", () => {
       serverFilePath: path.join(tmpDir, ".next", "standalone", "server.js"),
     };
     defaultNextVersion = "14.0.3";
+    adapterMetadata = {
+      adapterPackageName: "@apphosting/adapter-nextjs",
+      adapterVersion: "14.0.1",
+    };
   });
 
   it("expects all output bundle files to be generated", async () => {
-    const { generateBuildOutput, validateOutputDirectory, getAdapterMetadata } = await importUtils;
+    const { generateBuildOutput, validateOutputDirectory } = await importUtils;
     const files = {
       ".next/standalone/server.js": "",
       ".next/static/staticfile": "",
@@ -34,7 +40,6 @@ describe("build commands", () => {
         "redirects":[]
       }`,
     };
-    const adapterMetadata = getAdapterMetadata();
     generateTestFiles(tmpDir, files);
     await generateBuildOutput(
       tmpDir,
@@ -98,10 +103,7 @@ metadata:
       },
       path.join(tmpDir, ".next"),
       defaultNextVersion,
-      {
-        adapterPackageName: "@apphosting/adapter-nextjs",
-        adapterVersion: "14.0.1",
-      },
+      adapterMetadata,
     );
 
     const expectedFiles = {

packages/@apphosting/adapter-nextjs/src/interfaces.ts

@@ -1,4 +1,6 @@
 import type { RouteHas } from "next/dist/lib/load-custom-routes.js";
+import type { AssetBinding } from "next/dist/build/webpack/loaders/get-module-build-info.js";
+import type { MiddlewareMatcher } from "next/dist/build//analysis/get-page-static-info.js";
 
 export interface RoutesManifestRewriteObject {
   beforeFiles?: RoutesManifestRewrite[];
@@ -113,23 +115,23 @@ export interface Metadata extends AdapterMetadata {
   frameworkVersion: string;
 }
 
-// Metadata schema for asset bindings
-export interface AssetBinding {
-  filePath: string;
-  name: string;
-}
-
-// Metadata schema for middleware matchers
-export interface MiddlewareMatcher {
-  regexp: string;
-  locale?: false;
-  has?: RouteHas[];
-  missing?: RouteHas[];
-  originalSource: string;
+/* 
+  Next.js exposed internal interface for middleware manifest (middleware-manifest.json)
+  https://github.com/vercel/next.js/blob/v15.2.0-canary.76/packages/next/src/build/webpack/plugins/middleware-plugin.ts#L54
+*/
+export interface MiddlewareManifest {
+  version: 3;
+  sortedMiddleware: string[];
+  middleware: {
+    [page: string]: EdgeFunctionDefinition;
+  };
+  functions: {
+    [page: string]: EdgeFunctionDefinition;
+  };
 }
 
-// Metadata schema for edge function definitions
-export interface EdgeFunctionDefinition {
+// Next.js exposed internal interface for edge function definitions
+interface EdgeFunctionDefinition {
   files: string[];
   name: string;
   page: string;
@@ -138,15 +140,3 @@ export interface EdgeFunctionDefinition {
   assets?: AssetBinding[];
   regions?: string[] | string;
 }
-
-// Metadata schema for middleware manifest
-export interface MiddlewareManifest {
-  version: number;
-  sortedMiddleware: string[];
-  middleware: {
-    [page: string]: EdgeFunctionDefinition;
-  };
-  functions: {
-    [page: string]: EdgeFunctionDefinition;
-  };
-}

packages/@apphosting/adapter-nextjs/src/overrides.spec.ts

@@ -54,14 +54,35 @@ describe("route overrides", () => {
       fs.readFileSync(routesManifestPath, "utf-8"),
     ) as RoutesManifest;
 
-    assert.strictEqual(updatedManifest.headers.length, 2);
-    assert.deepStrictEqual(updatedManifest.headers[0], initialManifest.headers[0]);
-
-    const firebaseHeaders = updatedManifest.headers[1];
-    assert.strictEqual(firebaseHeaders.source, "/:path*");
-    assert.strictEqual(firebaseHeaders.headers.length, 1);
-    assert.strictEqual(firebaseHeaders.headers[0].key, "x-fah-adapter");
-    assert.strictEqual(firebaseHeaders.headers[0].value, "nextjs-1.0.0");
+    const expectedManifest: RoutesManifest = {
+      version: 3,
+      basePath: "",
+      pages404: true,
+      staticRoutes: [],
+      dynamicRoutes: [],
+      dataRoutes: [],
+      redirects: [],
+      rewrites: [],
+      headers: [
+        {
+          source: "/existing",
+          headers: [{ key: "X-Custom", value: "test" }],
+          regex: "^/existing$",
+        },
+        {
+          source: "/:path*",
+          regex: "^(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))?(?:/)?$",
+          headers: [
+            {
+              key: "x-fah-adapter",
+              value: "nextjs-1.0.0",
+            },
+          ],
+        },
+      ],
+    };
+
+    assert.deepStrictEqual(updatedManifest, expectedManifest);
   });
 
   it("should add middleware header when middleware exists", async () => {
@@ -79,7 +100,7 @@ describe("route overrides", () => {
     };
 
     const middlewareManifest: MiddlewareManifest = {
-      version: 1,
+      version: 3,
       sortedMiddleware: ["/"],
       middleware: {
         "/": {
@@ -111,13 +132,31 @@ describe("route overrides", () => {
 
     assert.strictEqual(updatedManifest.headers.length, 1);
 
-    const headers = updatedManifest.headers[0];
-    assert.strictEqual(headers.source, "/:path*");
-    assert.strictEqual(headers.headers.length, 2);
-    assert.strictEqual(headers.headers[0].key, "x-fah-adapter");
-    assert.strictEqual(headers.headers[0].value, "nextjs-1.0.0");
-    assert.strictEqual(headers.headers[1].key, "x-fah-middleware");
-    assert.strictEqual(headers.headers[1].value, "true");
+    const expectedManifest: RoutesManifest = {
+      version: 3,
+      basePath: "",
+      pages404: true,
+      staticRoutes: [],
+      dynamicRoutes: [],
+      dataRoutes: [],
+      rewrites: [],
+      redirects: [],
+      headers: [
+        {
+          source: "/:path*",
+          regex: "^(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))?(?:/)?$",
+          headers: [
+            {
+              key: "x-fah-adapter",
+              value: "nextjs-1.0.0",
+            },
+            { key: "x-fah-middleware", value: "true" },
+          ],
+        },
+      ],
+    };
+
+    assert.deepStrictEqual(updatedManifest, expectedManifest);
   });
 
   afterEach(() => {

packages/@apphosting/adapter-nextjs/src/overrides.ts

@@ -2,7 +2,13 @@ import { AdapterMetadata, MiddlewareManifest } from "./interfaces.js";
 import { loadRouteManifest, writeRouteManifest, loadMiddlewareManifest } from "./utils.js";
 
 /**
- * Adds the route overrides to the route manifest.
+ * Modifies the app's route manifest (routes-manifest.json) to add Firebase App Hosting
+ * specific overrides (i.e headers).
+ *
+ * This function adds the following headers to all routes:
+ * - x-fah-adapter: The Firebase App Hosting adapter version used to build the app.
+ * - x-fah-middleware: When middleware is enabled.
+ *
  * @param appPath The path to the app directory.
  * @param distDir The path to the dist directory.
  * @param adapterMetadata The adapter metadata.
@@ -30,6 +36,14 @@ export async function addRouteOverrides(
           ]
         : []),
     ],
+    /* 
+      NextJs converts the source string to a regex using path-to-regexp (https://github.com/pillarjs/path-to-regexp) at 
+      build time: https://github.com/vercel/next.js/blob/canary/packages/next/src/build/index.ts#L1273.
+      This regex is then used to match the route against the request path.
+
+      This regex was generated by building a sample NextJs app with the source string `/:path*` and then inspecting the
+      routes-manifest.json file.
+    */
     regex: "^(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))?(?:/)?$",
   });