Skip to content

Commit

Permalink
fix(@angular/build): show error when Node.js built-ins are used durin…
Browse files Browse the repository at this point in the history
…g `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 06f478b)
  • Loading branch information
alan-agius4 committed Dec 5, 2024
1 parent 14451e2 commit fc41f50
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 21 deletions.
20 changes: 20 additions & 0 deletions packages/angular/build/src/builders/dev-server/vite-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' : ''
Expand Down
26 changes: 26 additions & 0 deletions tests/legacy-cli/e2e/tests/vite/browser-node-module-dep-error.ts
Original file line number Diff line number Diff line change
@@ -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/,
);
}
34 changes: 13 additions & 21 deletions tests/legacy-cli/e2e/tests/vite/reuse-dep-optimization-cache.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,35 @@
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');
await ng('cache', 'on');

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' },
);
}

0 comments on commit fc41f50

Please sign in to comment.