Only add x-fah-middleware header to routes that have middleware enabled (#325)

* only add x-fah-middleware routes to routes that have middleware enabled

* start tests

* fix bug and fix unit tests

* fixup e2e tests

* fix lint error

* fix e2e

* bump adapter version

Author: mathu97

package-lock.json

@@ -15,7 +15,7 @@ before(() => {
 });
 
 describe("middleware", () => {
-  it("should have x-fah-adapter header and x-fah-middleware header on all routes", async () => {
+  it("should have x-fah-adapter header on all routes", async () => {
     const routes = [
       "/",
       "/ssg",
@@ -33,6 +33,15 @@ describe("middleware", () => {
         `nextjs-${adapterVersion}`,
         `Route ${route} missing x-fah-adapter header`,
       );
+    }
+  });
+
+  it("should have x-fah-middleware header on middleware routes", async () => {
+    // Middleware is configured to run on these routes via run-local.ts with-middleware setup function
+    const routes = ["/ssg", "/ssr"];
+
+    for (const route of routes) {
+      const response = await fetch(posix.join(host, route));
       assert.equal(
         response.headers.get("x-fah-middleware"),
         "true",

packages/@apphosting/adapter-nextjs/e2e/middleware.spec.ts

@@ -35,15 +35,15 @@ const scenarios: Scenario[] = [
     setup: async (cwd: string) => {
       // Create a middleware.ts file
       const middlewareContent = `
-        import type { NextRequest } from 'next/server'
+        import type { NextRequest } from "next/server";
 
         export function middleware(request: NextRequest) {
           // This is a simple middleware that doesn't modify the request
-          console.log('Middleware executed', request.nextUrl.pathname);
+          console.log("Middleware executed", request.nextUrl.pathname);
         }
 
         export const config = {
-          matcher: '/((?!api|_next/static|_next/image|favicon.ico).*)',
+          matcher: ["/ssg", "/ssr"],
         };
       `;
 

packages/@apphosting/adapter-nextjs/e2e/run-local.ts

@@ -1,6 +1,6 @@
 {
   "name": "@apphosting/adapter-nextjs",
-  "version": "14.0.12",
+  "version": "14.0.13",
   "main": "dist/index.js",
   "description": "Experimental addon to the Firebase CLI to add web framework support",
   "repository": {

packages/@apphosting/adapter-nextjs/package.json

@@ -85,7 +85,7 @@ describe("route overrides", () => {
     assert.deepStrictEqual(updatedManifest, expectedManifest);
   });
 
-  it("should add middleware header when middleware exists", async () => {
+  it("should add middleware header only to routes for which middleware is enabled", async () => {
     const { addRouteOverrides } = await importOverrides;
     const initialManifest: RoutesManifest = {
       version: 3,
@@ -109,8 +109,8 @@ describe("route overrides", () => {
           page: "/",
           matchers: [
             {
-              regexp: "^/.*$",
-              originalSource: "/:path*",
+              regexp: "/hello",
+              originalSource: "/hello",
             },
           ],
         },
@@ -130,7 +130,7 @@ describe("route overrides", () => {
       fs.readFileSync(routesManifestPath, "utf-8"),
     ) as RoutesManifest;
 
-    assert.strictEqual(updatedManifest.headers.length, 1);
+    assert.strictEqual(updatedManifest.headers.length, 2);
 
     const expectedManifest: RoutesManifest = {
       version: 3,
@@ -150,9 +150,13 @@ describe("route overrides", () => {
               key: "x-fah-adapter",
               value: "nextjs-1.0.0",
             },
-            { key: "x-fah-middleware", value: "true" },
           ],
         },
+        {
+          source: "/hello",
+          regex: "/hello",
+          headers: [{ key: "x-fah-middleware", value: "true" }],
+        },
       ],
     };
 
@@ -172,13 +176,13 @@ describe("next config overrides", () => {
     ...config,
     images: {
       ...(config.images || {}),
-      ...(config.images?.unoptimized === undefined && config.images?.loader === undefined 
-          ? { unoptimized: true } 
+      ...(config.images?.unoptimized === undefined && config.images?.loader === undefined
+          ? { unoptimized: true }
           : {}),
     },
   });
 
-  const config = typeof originalConfig === 'function' 
+  const config = typeof originalConfig === 'function'
     ? async (...args) => {
         const resolvedConfig = await originalConfig(...args);
         return fahOptimizedConfig(resolvedConfig);
@@ -194,12 +198,12 @@ describe("next config overrides", () => {
     const { overrideNextConfig } = await importOverrides;
     const originalConfig = `
     // @ts-check
- 
+
     /** @type {import('next').NextConfig} */
     const nextConfig = {
       /* config options here */
     }
-    
+
     module.exports = nextConfig
     `;
 
@@ -213,7 +217,7 @@ describe("next config overrides", () => {
       normalizeWhitespace(`
       // @ts-nocheck
       const originalConfig = require('./next.config.original.js');
-      
+
       ${nextConfigOverrideBody}
 
       module.exports = config;
@@ -225,14 +229,14 @@ describe("next config overrides", () => {
     const { overrideNextConfig } = await importOverrides;
     const originalConfig = `
     // @ts-check
-    
+
     /**
      * @type {import('next').NextConfig}
      */
     const nextConfig = {
       /* config options here */
     }
-    
+
     export default nextConfig
     `;
 
@@ -257,7 +261,7 @@ describe("next config overrides", () => {
     const { overrideNextConfig } = await importOverrides;
     const originalConfig = `
     // @ts-check
-    
+
     export default (phase, { defaultConfig }) => {
       /**
        * @type {import('next').NextConfig}
@@ -280,7 +284,7 @@ describe("next config overrides", () => {
       import originalConfig from './next.config.original.mjs';
 
       ${nextConfigOverrideBody}
-      
+
       export default config;
       `),
     );
@@ -290,11 +294,11 @@ describe("next config overrides", () => {
     const { overrideNextConfig } = await importOverrides;
     const originalConfig = `
     import type { NextConfig } from 'next'
-    
+
     const nextConfig: NextConfig = {
       /* config options here */
     }
-    
+
     export default nextConfig
     `;
 
@@ -307,9 +311,9 @@ describe("next config overrides", () => {
       normalizeWhitespace(`
       // @ts-nocheck
       import originalConfig from './next.config.original';
-      
+
       ${nextConfigOverrideBody}
-      
+
       module.exports = config;
       `),
     );

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

@@ -1,4 +1,4 @@
-import { AdapterMetadata, MiddlewareManifest } from "./interfaces.js";
+import { AdapterMetadata } from "./interfaces.js";
 import {
   loadRouteManifest,
   writeRouteManifest,
@@ -146,9 +146,12 @@ export async function validateNextConfigOverride(
  * 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:
+ * It adds the following headers to all routes:
  * - x-fah-adapter: The Firebase App Hosting adapter version used to build the app.
+ *
+ * It also adds the following headers to all routes for which middleware is enabled:
  * - 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.
@@ -158,38 +161,47 @@ export async function addRouteOverrides(
   distDir: string,
   adapterMetadata: AdapterMetadata,
 ) {
-  const middlewareManifest = loadMiddlewareManifest(appPath, distDir);
   const routeManifest = loadRouteManifest(appPath, distDir);
+
+  // Add the adapter version to all routes
   routeManifest.headers.push({
     source: "/:path*",
     headers: [
       {
         key: "x-fah-adapter",
         value: `nextjs-${adapterMetadata.adapterVersion}`,
       },
-      ...(middlewareExists(middlewareManifest)
-        ? [
-            {
-              key: "x-fah-middleware",
-              value: "true",
-            },
-          ]
-        : []),
     ],
     /*
-      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.
+    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.
-    */
+    This regex was generated by building a sample NextJs app with the source string `/:path*` and then inspecting the
+    routes-manifest.json file.
+  */
     regex: "^(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))?(?:/)?$",
   });
 
-  await writeRouteManifest(appPath, distDir, routeManifest);
-}
+  // Add the middleware header to all routes for which middleware is enabled
+  const middlewareManifest = loadMiddlewareManifest(appPath, distDir);
+  const rootMiddleware = middlewareManifest.middleware["/"];
+  if (rootMiddleware?.matchers) {
+    console.log("Middleware detected, adding middleware headers to matching routes");
+
+    rootMiddleware.matchers.forEach((matcher) => {
+      routeManifest.headers.push({
+        source: matcher.originalSource,
+        headers: [
+          {
+            key: "x-fah-middleware",
+            value: "true",
+          },
+        ],
+        regex: matcher.regexp,
+      });
+    });
+  }
 
-function middlewareExists(middlewareManifest: MiddlewareManifest) {
-  return Object.keys(middlewareManifest.middleware).length > 0;
+  await writeRouteManifest(appPath, distDir, routeManifest);
 }