From fc41f50b53bbffead017b420105eed5bd8573ac1 Mon Sep 17 00:00:00 2001
From: Alan Agius <alanagius@google.com>
Date: Thu, 5 Dec 2024 16:55:26 +0000
Subject: [PATCH] fix(@angular/build): show error when Node.js built-ins are
 used during `ng serve`

This commit ensures consistent behavior between `ng build` and `ng serve`. Previously, `ng serve` did not display an error message when Node.js built-in modules were included in browser bundles. By default, Vite replaces Node.js built-ins with empty modules, which can lead to unexpected runtime issues. This update addresses the problem by surfacing clear error messages, providing better developer feedback and alignment between the two commands.

Closes: #27425
(cherry picked from commit 06f478bc18d3e0daa8902d0fef94e55a5d052348)
---
 .../src/builders/dev-server/vite-server.ts    | 20 +++++++++++
 .../vite/browser-node-module-dep-error.ts     | 26 ++++++++++++++
 .../vite/reuse-dep-optimization-cache.ts      | 34 +++++++------------
 3 files changed, 59 insertions(+), 21 deletions(-)
 create mode 100644 tests/legacy-cli/e2e/tests/vite/browser-node-module-dep-error.ts

diff --git a/packages/angular/build/src/builders/dev-server/vite-server.ts b/packages/angular/build/src/builders/dev-server/vite-server.ts
index dfbbc8454b53..832002953fc1 100644
--- a/packages/angular/build/src/builders/dev-server/vite-server.ts
+++ b/packages/angular/build/src/builders/dev-server/vite-server.ts
@@ -819,6 +819,26 @@ function getDepOptimizationConfig({
   thirdPartySourcemaps: boolean;
 }): DepOptimizationConfig {
   const plugins: ViteEsBuildPlugin[] = [
+    {
+      name: 'angular-browser-node-built-in',
+      setup(build) {
+        // This namespace is configured by vite.
+        // @see: https://github.com/vitejs/vite/blob/a1dd396da856401a12c921d0cd2c4e97cb63f1b5/packages/vite/src/node/optimizer/esbuildDepPlugin.ts#L109
+        build.onLoad({ filter: /.*/, namespace: 'browser-external' }, (args) => {
+          if (!isBuiltin(args.path)) {
+            return;
+          }
+
+          return {
+            errors: [
+              {
+                text: `The package "${args.path}" wasn't found on the file system but is built into node.`,
+              },
+            ],
+          };
+        });
+      },
+    },
     {
       name: `angular-vite-optimize-deps${ssr ? '-ssr' : ''}${
         thirdPartySourcemaps ? '-vendor-sourcemap' : ''
diff --git a/tests/legacy-cli/e2e/tests/vite/browser-node-module-dep-error.ts b/tests/legacy-cli/e2e/tests/vite/browser-node-module-dep-error.ts
new file mode 100644
index 000000000000..7d2f45198c7c
--- /dev/null
+++ b/tests/legacy-cli/e2e/tests/vite/browser-node-module-dep-error.ts
@@ -0,0 +1,26 @@
+import assert from 'node:assert';
+import { execAndWaitForOutputToMatch, ng } from '../../utils/process';
+import { writeFile } from '../../utils/fs';
+import { getGlobalVariable } from '../../utils/env';
+
+export default async function () {
+  assert(
+    getGlobalVariable('argv')['esbuild'],
+    'This test should not be called in the Webpack suite.',
+  );
+
+  await ng('cache', 'clean');
+  await ng('cache', 'on');
+
+  await writeFile('src/main.ts', `import '@angular-devkit/core/node';`);
+
+  const { stderr } = await execAndWaitForOutputToMatch('ng', ['serve'], /ERROR.+node:path/, {
+    CI: '0',
+    NO_COLOR: 'true',
+  });
+
+  assert.match(
+    stderr,
+    /The package "node:path" wasn't found on the file system but is built into node/,
+  );
+}
diff --git a/tests/legacy-cli/e2e/tests/vite/reuse-dep-optimization-cache.ts b/tests/legacy-cli/e2e/tests/vite/reuse-dep-optimization-cache.ts
index f4b93ccfcd2f..c7c03be87e2c 100644
--- a/tests/legacy-cli/e2e/tests/vite/reuse-dep-optimization-cache.ts
+++ b/tests/legacy-cli/e2e/tests/vite/reuse-dep-optimization-cache.ts
@@ -1,11 +1,7 @@
 import assert from 'node:assert';
+import { setTimeout } from 'node:timers/promises';
 import { findFreePort } from '../../utils/network';
-import {
-  execAndWaitForOutputToMatch,
-  killAllProcesses,
-  ng,
-  waitForAnyProcessOutputToMatch,
-} from '../../utils/process';
+import { execAndWaitForOutputToMatch, killAllProcesses, ng } from '../../utils/process';
 
 export default async function () {
   await ng('cache', 'clean');
@@ -13,31 +9,27 @@ export default async function () {
 
   const port = await findFreePort();
 
-  // Make sure serve is consistent with build
-  await execAndWaitForOutputToMatch(
-    'ng',
-    ['serve', '--port', `${port}`],
-    /Dependencies bundled/,
-    // Use CI:0 to force caching
-    { DEBUG: 'vite:deps', CI: '0' },
-  );
+  const [, response] = await Promise.all([
+    execAndWaitForOutputToMatch(
+      'ng',
+      ['serve', '--port', `${port}`],
+      /dependencies optimized/,
+      // Use CI:0 to force caching
+      { DEBUG: 'vite:deps', CI: '0', NO_COLOR: 'true' },
+    ),
+    setTimeout(4_000).then(() => fetch(`http://localhost:${port}/main.js`)),
+  ]);
 
-  // Make request so that vite writes the cache.
-  const response = await fetch(`http://localhost:${port}/main.js`);
   assert(response.ok, `Expected 'response.ok' to be 'true'.`);
 
-  // Wait for vite to write to FS and stablize.
-  await waitForAnyProcessOutputToMatch(/dependencies optimized/, 5000);
-
   // Terminate the dev-server
   await killAllProcesses();
 
-  // The Node.js specific module should not be found
   await execAndWaitForOutputToMatch(
     'ng',
     ['serve', '--port=0'],
     /Hash is consistent\. Skipping/,
     // Use CI:0 to force caching
-    { DEBUG: 'vite:deps', CI: '0' },
+    { DEBUG: 'vite:deps', CI: '0', NO_COLOR: 'true' },
   );
 }