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: angular#27425
  • Loading branch information
alan-agius4 committed Dec 5, 2024
1 parent f717a54 commit 06f478b
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 @@ -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' : ''
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/, {
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,35 +1,27 @@
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}`],
/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();

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

0 comments on commit 06f478b

Please sign in to comment.