Repository: firebase/firebase-tools (ts)

Issue:
Deploying NextJS using Firebase Deploy throws "Cannot read properties of null (reading 'toString')     at ɵcodegenFunctionsDirectory "
<!-- DO NOT DELETE
validate_template=true
template_path=.github/ISSUE_TEMPLATE/bug_report.md
-->

<!--
Thank you for contributing to the Firebase community!

Think you found a bug?
=======================
Yeah, we're definitely not perfect! Please use this template and include a minimal repro when opening the issue. If you know how to solve the issue, please create a Pull Request, and we'd be happy to review it!

Have a feature request?
========================
Great, we love hearing how we can improve our products! However, do not use this template to submit a feature request. Please submit your feature requests to: https://firebase.google.com/support/contact/bugs-features/

Have a usage question?
=======================
We get lots of those and we love helping you, but GitHub is not the best place for them and they will be closed. Please take a look at the guide first: https://firebase.google.com/docs/cli/

If the official documentation doesn't help, try asking through our official support channel: https://firebase.google.com/support/

Additional locations to check for solutions or assistance from the community:
- Stack Overflow: https://stackoverflow.com/
- Firebase Slack Community: https://firebase.community/

*Please avoid duplicate posting across multiple channels!*
-->

### [REQUIRED] Environment info

<!-- What version of the Firebase CLI (`firebase-tools`) are you using? Note that your issue may already be fixed in the latest versions. The latest version can be found at https://github.com/firebase/firebase-tools/releases -->
Firebase: 9.15.0
Node.js: 18.0.0
Npm: 8.6.0


<!-- Output of `firebase --version` -->

firebase-tools: 11.19.0

<!-- e.g. macOS, Windows, Ubuntu -->

Platform: Windows

### [REQUIRED] Test case

Deploy NextJS using Firebase deploy using the new launched experimental feature for web frameworks

### [REQUIRED] Steps to reproduce

1. Create a project using "npx create-next-app@latest"
2. Install the latest version of Firebase CLI
3.  cd into the NextJS project and "npm install firebase"
4. Create a .env.local file to add all the environment variables
5.  Firebase init --> choose hosting & functions
6.  npm run dev to test if everything is working fine locally (everything works)

Package.json

{
  "name": "gpt3-cover-letter",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@next/font": "13.1.1",
    "eslint": "8.30.0",
    "eslint-config-next": "13.1.1",
    "file-saver": "^2.0.5",
    "firebase": "^9.15.0",
    "next": "^13.1.1",
    "openai": "^3.1.0",
    "react": "18.2.0",
    "react-dom": "18.2.0"
  }
}

Firebase.json

{
  "functions": [
    {
      "source": "functions",
      "codebase": "default",
      "ignore": [
        "node_modules",
        ".git",
        "firebase-debug.log",
        "firebase-debug.*.log"
      ]
    }
  ],
  "hosting": {
    "source": ".",
    "ignore": [
      "firebase.json",
      "**/.*",
      "**/node_modules/**"
    ]
  },
  "emulators": {
    "functions": {
      "port": 5001
    },
    "hosting": {
      "port": 5000
    },
    "ui": {
      "enabled": true
    },
    "singleProjectMode": true,
    "auth": {
      "port": 9099
    },
    "firestore": {
      "port": 8080
    },
    "database": {
      "port": 9000
    },
    "pubsub": {
      "port": 8085
    },
    "storage": {
      "port": 9199
    },
    "eventarc": {
      "port": 9299
    }
  }
}


### [REQUIRED] Expected behavior

When I run "firebase deploy", my NextJS is deployed.

### [REQUIRED] Actual behavior

I get the following error when running "firebase deploy"

TypeError: Cannot read properties of null (reading 'toString')
    at ɵcodegenFunctionsDirectory (C:\Users\sanyo\AppData\Roaming\npm\node_modules\firebase-tools\lib\frameworks\next\index.js:220:18)
    at async prepareFrameworks (C:\Users\sanyo\AppData\Roaming\npm\node_modules\firebase-tools\lib\frameworks\index.js:305:84)
    at async deploy (C:\Users\sanyo\AppData\Roaming\npm\node_modules\firebase-tools\lib\deploy\index.js:53:13)


Pull request description:
Fix Next.js deployments on Windows
<!--

Thank you for contributing to the Firebase community! Please fill out the form below.

Run the linter and test suite
==============================
Run `npm test` to make sure your changes compile properly and the tests all pass on your local machine. We've hooked up this repo with continuous integration to double check those things for you.

-->

### Description

Wrapping up @jamesdaniels PR #5393 aiming to fix #5369. Ended up that the error was only happening on Windows due to `spawn` executing commands which were not recognized by the terminal. The fix was to replace `spawn` and `spawnSync` with its equivalent from [`cross-spawn`](https://www.npmjs.com/package/cross-spawn), which is a cross platform implementation of `spawn` that's [already used in `firebase-tools`](https://github.com/firebase/firebase-tools/blob/master/package.json#L111).
<!-- Are you fixing a bug? Implementing a new feature? Make sure we have the context around your change. Link to other relevant issues or pull requests. -->

### Scenarios Tested

Tested this on both MacOs and Windows devices and `firebase deploy` worked following these steps:

1. Create a project using `npx create-next-app@latest`
2. Build this branch with `npm run build`
3. From the Next.js project folder, run `node ../firebase-tools/lib/bin/firebase.js init hosting`
4. `node ../firebase-tools/lib/bin/firebase.js deploy`


<!-- Write a list of all the user journeys and edge cases you've tested. Instructions for manual testing can be found at https://github.com/firebase/firebase-tools/blob/master/.github/CONTRIBUTING.md#development-setup -->

### Sample Commands

--
<!-- Proposing a change to commands or flags? Provide examples of how they will be used. -->


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 316c2d80c..371875394 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,3 +11,4 @@
 - Fix various accessibility and usability issues in Emulator UI.
 - Support .env when deploying a web framework (#5501)
 - Fix various issues with "init hosting" and web frameworks (#5500)
+- Fix Next.js deployments on Windows (#5499)
diff --git a/src/frameworks/angular/index.ts b/src/frameworks/angular/index.ts
index 13f52e108..4f6dcbb8f 100644
--- a/src/frameworks/angular/index.ts
+++ b/src/frameworks/angular/index.ts
@@ -1,6 +1,7 @@
 import type { Target } from "@angular-devkit/architect";
 import { join } from "path";
-import { execSync, spawn } from "child_process";
+import { execSync } from "child_process";
+import { spawn } from "cross-spawn";
 import { copy, pathExists } from "fs-extra";
 import { mkdir } from "fs/promises";
 
diff --git a/src/frameworks/index.ts b/src/frameworks/index.ts
index e575ab65e..a132536f2 100644
--- a/src/frameworks/index.ts
+++ b/src/frameworks/index.ts
@@ -1,6 +1,7 @@
 import { join, relative, extname, basename } from "path";
 import { exit } from "process";
-import { execSync, spawnSync } from "child_process";
+import { execSync } from "child_process";
+import { sync as spawnSync } from "cross-spawn";
 import { readdirSync, statSync } from "fs";
 import { pathToFileURL } from "url";
 import { IncomingMessage, ServerResponse } from "http";
diff --git a/src/frameworks/next/index.ts b/src/frameworks/next/index.ts
index 8a5e126b4..f03dd4f5f 100644
--- a/src/frameworks/next/index.ts
+++ b/src/frameworks/next/index.ts
@@ -1,4 +1,5 @@
-import { execSync, spawnSync } from "child_process";
+import { execSync } from "child_process";
+import { spawn, sync as spawnSync } from "cross-spawn";
 import { mkdir, copyFile } from "fs/promises";
 import { dirname, join } from "path";
 import type { NextConfig } from "next";
@@ -11,6 +12,10 @@ import { existsSync } from "fs";
 import { gte } from "semver";
 import { IncomingMessage, ServerResponse } from "http";
 import * as clc from "colorette";
+import { chain } from "stream-chain";
+import { parser } from "stream-json";
+import { pick } from "stream-json/filters/Pick";
+import { streamObject } from "stream-json/streamers/StreamObject";
 
 import {
   BuildResult,
@@ -34,7 +39,7 @@ import {
   isUsingMiddleware,
   allDependencyNames,
 } from "./utils";
-import type { Manifest, NpmLsReturn } from "./interfaces";
+import type { Manifest, NpmLsDepdendency } from "./interfaces";
 import { readJSON } from "../utils";
 import { warnIfCustomBuildScript } from "../utils";
 import type { EmulatorInfo } from "../../emulator/types";
@@ -345,33 +350,52 @@ export async function ɵcodegenPublicDirectory(sourceDir: string, destDir: strin
 export async function ɵcodegenFunctionsDirectory(sourceDir: string, destDir: string) {
   const { distDir } = await getConfig(sourceDir);
   const packageJson = await readJSON(join(sourceDir, "package.json"));
+  // Bundle their next.config.js with esbuild via NPX, pinned version was having troubles on m1
+  // macs and older Node versions; either way, we should avoid taking on any deps in firebase-tools
+  // Alternatively I tried using @swc/spack and the webpack bundled into Next.js but was
+  // encountering difficulties with both of those
   if (existsSync(join(sourceDir, "next.config.js"))) {
-    // Bundle their next.config.js with esbuild via NPX, pinned version was having troubles on m1
-    // macs and older Node versions; either way, we should avoid taking on any deps in firebase-tools
-    // Alternatively I tried using @swc/spack and the webpack bundled into Next.js but was
-    // encountering difficulties with both of those
-    const dependencyTree: NpmLsReturn = JSON.parse(
-      spawnSync("npm", ["ls", "--omit=dev", "--all", "--json"], {
+    try {
+      const productionDeps = await new Promise<string[]>((resolve) => {
+        const dependencies: string[] = [];
+        const pipeline = chain([
+          spawn("npm", ["ls", "--omit=dev", "--all", "--json"], { cwd: sourceDir }).stdout,
+          parser({ packValues: false, packKeys: true, streamValues: false }),
+          pick({ filter: "dependencies" }),
+          streamObject(),
+          ({ key, value }: { key: string; value: NpmLsDepdendency }) => [
+            key,
+            ...allDependencyNames(value),
+          ],
+        ]);
+        pipeline.on("data", (it: string) => dependencies.push(it));
+        pipeline.on("end", () => {
+          resolve([...new Set(dependencies)]);
+        });
+      });
+      // Mark all production deps as externals, so they aren't bundled
+      // DevDeps won't be included in the Cloud Function, so they should be bundled
+      const esbuildArgs = productionDeps
+        .map((it) => `--external:${it}`)
+        .concat(
+          "--bundle",
+          "--platform=node",
+          `--target=node${NODE_VERSION}`,
+          `--outdir=${destDir}`,
+          "--log-level=error"
+        );
+      const bundle = spawnSync("npx", ["--yes", "esbuild", "next.config.js", ...esbuildArgs], {
         cwd: sourceDir,
-      }).stdout.toString()
-    );
-    // Mark all production deps as externals, so they aren't bundled
-    // DevDeps won't be included in the Cloud Function, so they should be bundled
-    const esbuildArgs = allDependencyNames(dependencyTree)
-      .map((it) => `--external:${it}`)
-      .concat(
-        "--bundle",
-        "--platform=node",
-        `--target=node${NODE_VERSION}`,
-        `--outdir=${destDir}`,
-        "--log-level=error"
+      });
+      if (bundle.status) {
+        throw new FirebaseError(bundle.stderr.toString());
+      }
+    } catch (e: any) {
+      console.warn(
+        "Unable to bundle next.config.js for use in Cloud Functions, proceeding with deploy but problems may be enountered."
       );
-    const bundle = spawnSync("npx", ["--yes", "esbuild", "next.config.js", ...esbuildArgs], {
-      cwd: sourceDir,
-    });
-    if (bundle.status) {
-      console.error(bundle.stderr.toString());
-      throw new FirebaseError("Unable to bundle next.config.js for use in Cloud Functions");
+      console.error(e.message);
+      copy(join(sourceDir, "next.config.js"), join(destDir, "next.config.js"));
     }
   }
   if (await pathExists(join(sourceDir, "public"))) {
diff --git a/src/frameworks/next/utils.ts b/src/frameworks/next/utils.ts
index 793a8cd12..7ba2faeb6 100644
--- a/src/frameworks/next/utils.ts
+++ b/src/frameworks/next/utils.ts
@@ -229,6 +229,5 @@ export function allDependencyNames(mod: NpmLsDepdendency): string[] {
     (acc, it) => [...acc, it, ...allDependencyNames(mod.dependencies![it])],
     [] as string[]
   );
-  // deduplicate the names
-  return [...new Set(dependencyNames)];
+  return dependencyNames;
 }
diff --git a/src/frameworks/vite/index.ts b/src/frameworks/vite/index.ts
index 21f0f6f5e..9979c2014 100644
--- a/src/frameworks/vite/index.ts
+++ b/src/frameworks/vite/index.ts
@@ -1,4 +1,5 @@
-import { execSync, spawn } from "child_process";
+import { execSync } from "child_process";
+import { spawn } from "cross-spawn";
 import { existsSync } from "fs";
 import { copy, pathExists } from "fs-extra";
 import { join } from "path";


Test patch:
diff --git a/src/test/frameworks/next/utils.spec.ts b/src/test/frameworks/next/utils.spec.ts
index 31d5877dc..df19ef171 100644
--- a/src/test/frameworks/next/utils.spec.ts
+++ b/src/test/frameworks/next/utils.spec.ts
@@ -353,9 +353,15 @@ describe("Next.js utils", () => {
         "sass",
         "styled-jsx",
         "client-only",
+        "react",
+        "react-dom",
         "loose-envify",
         "js-tokens",
+        "react",
         "scheduler",
+        "loose-envify",
+        "react",
+        "loose-envify",
       ]);
     });
   });