Merge branch 'main' into feat/support-non-existent-next-configs

Author: mathu97

CONTRIBUTING.md

@@ -25,4 +25,4 @@ information on using pull requests.
 ## Community Guidelines
 
 This project follows [Google's Open Source Community
-Guidelines](https://opensource.google/conduct/).
+Guidelines](https://opensource.google/conduct/).

package-lock.json

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

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

@@ -50,40 +50,75 @@ export async function checkBuildConditions(opts: BuildOptions): Promise<void> {
     return;
   }
 
+  let angularBuilder = "";
   // dynamically load Angular so this can be used in an NPX context
   const angularCorePath = require.resolve("@angular/core", { paths: [process.cwd()] });
-  const { NodeJsAsyncHost }: typeof import("@angular-devkit/core/node") = await import(
-    require.resolve("@angular-devkit/core/node", {
-      paths: [process.cwd(), angularCorePath],
-    })
-  );
-  const { workspaces }: typeof import("@angular-devkit/core") = await import(
-    require.resolve("@angular-devkit/core", {
-      paths: [process.cwd(), angularCorePath],
-    })
-  );
-  const host = workspaces.createWorkspaceHost(new NodeJsAsyncHost());
-  const { workspace } = await workspaces.readWorkspace(opts.projectDirectory, host);
-
-  const apps: string[] = [];
-  workspace.projects.forEach((value, key) => {
-    if (value.extensions.projectType === "application") apps.push(key);
-  });
-  const project = apps[0];
-  if (apps.length > 1 || !project) throw new Error("Unable to determine the application to deploy");
-
-  const workspaceProject = workspace.projects.get(project);
-  if (!workspaceProject) throw new Error(`No project ${project} found.`);
-
-  const target = "build";
-  if (!workspaceProject.targets.has(target)) throw new Error("Could not find build target.");
-
-  const { builder } = workspaceProject.targets.get(target)!;
-  if (!ALLOWED_BUILDERS.includes(builder)) {
-    throw new Error(
-      `Currently, only the following builders are supported: ${ALLOWED_BUILDERS.join(",")}.`,
+  try {
+    // Note: we assume that the user's app has @angular-devkit/core in their node_modules/
+    // because we expect them to have @angular-devkit/build-angular as a dependency which
+    // pulls in @angular-devkit/core as a dependency. However this assumption may not hold
+    // due to tree shaking.
+    const { NodeJsAsyncHost }: typeof import("@angular-devkit/core/node") = await import(
+      require.resolve("@angular-devkit/core/node", {
+        paths: [process.cwd(), angularCorePath],
+      })
     );
+    const { workspaces }: typeof import("@angular-devkit/core") = await import(
+      require.resolve("@angular-devkit/core", {
+        paths: [process.cwd(), angularCorePath],
+      })
+    );
+    const host = workspaces.createWorkspaceHost(new NodeJsAsyncHost());
+    const { workspace } = await workspaces.readWorkspace(opts.projectDirectory, host);
+
+    const apps: string[] = [];
+    workspace.projects.forEach((value, key) => {
+      if (value.extensions.projectType === "application") apps.push(key);
+    });
+    const project = apps[0];
+    if (apps.length > 1 || !project) {
+      throw new Error("Unable to determine the application to deploy");
+    }
+
+    const workspaceProject = workspace.projects.get(project);
+    if (!workspaceProject) {
+      throw new Error(`No project ${project} found.`);
+    }
+
+    const target = "build";
+    if (!workspaceProject.targets.has(target)) throw new Error("Could not find build target.");
+
+    const { builder } = workspaceProject.targets.get(target)!;
+    angularBuilder = builder;
+  } catch (error) {
+    logger.debug("failed to determine angular builder from the workspace api: ", error);
+    try {
+      const root = process.cwd();
+      const angularJSON = JSON.parse(readFileSync(join(root, "angular.json")).toString());
+      const apps: string[] = [];
+      Object.keys(angularJSON.projects).forEach((projectName) => {
+        const project = angularJSON.projects[projectName];
+        if (project["projectType"] === "application") apps.push(projectName);
+      });
+      const project = apps[0];
+      if (apps.length > 1 || !project)
+        throw new Error("Unable to determine the application to deploy");
+      angularBuilder = angularJSON.projects[project].architect.build.builder;
+    } catch (error) {
+      logger.debug("failed to determine angular builder from parsing angular.json: ", error);
+    }
+  }
+
+  if (angularBuilder !== "") {
+    if (!ALLOWED_BUILDERS.includes(angularBuilder)) {
+      throw new Error(
+        `Currently, only the following builders are supported: ${ALLOWED_BUILDERS.join(",")}.`,
+      );
+    }
   }
+  // This is just a validation step and our methods for validation are flakey. If we failed to extract
+  // the angular builder for validation, the build will continue. It may fail further down the line
+  // but the failure reason should be non-ambigious.
 }
 
 // Populate file or directory paths we need for generating output directory

packages/@apphosting/adapter-angular/src/utils.ts

@@ -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" }],
+        },
       ],
     };
 

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,
@@ -175,9 +175,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.
@@ -187,38 +190,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);
 }