From 06f478bc18d3e0daa8902d0fef94e55a5d052348 Mon Sep 17 00:00:00 2001
From: Alan Agius <alanagius@google.com>
Date: Thu, 5 Dec 2024 16:28:30 +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
---
 .../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 39b27b2bd6c7..64c68fd26888 100644
--- a/packages/angular/build/src/builders/dev-server/vite-server.ts
+++ b/packages/angular/build/src/builders/dev-server/vite-server.ts
@@ -803,6 +803,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..8aa791750c23
--- /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/, {
+    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 26caf5a9afb3..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,23 +9,19 @@ export default async function () {
 
   const port = await findFreePort();
 
-  // Make sure serve is consistent with build
-  await execAndWaitForOutputToMatch(
-    'ng',
-    ['serve', '--port', `${port}`],
-    /bundle generation complete/,
-    // Use CI:0 to force caching
-    { DEBUG: 'vite:deps', CI: '0', NO_COLOR: 'true' },
-  );
-
-  // Wait for vite to write to FS and stablize.
-  await Promise.all([
-    waitForAnyProcessOutputToMatch(/dependencies optimized/, 5000),
-    fetch(`http://localhost:${port}/main.js`).then((r) =>
-      assert(r.ok, `Expected 'response.ok' to be 'true'.`),
+  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`)),
   ]);
 
+  assert(response.ok, `Expected 'response.ok' to be 'true'.`);
+
   // Terminate the dev-server
   await killAllProcesses();
 
@@ -38,6 +30,6 @@ export default async function () {
     ['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' },
   );
 }