Skip to content

fix(@angular/build): Proper shutdown of esbuild workers #33202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aukevanoost wants to merge 1 commit into
base: main
Choose a base branch
from
+9 −1
Open

fix(@angular/build): Proper shutdown of esbuild workers #33202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/angular/build/src/builders/application/build-action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { BuilderContext } from '@angular-devkit/architect';
import { existsSync } from 'node:fs';
import path from 'node:path';
import { shutdownEsbuild } from '../../tools/esbuild/bundler-context';
import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-files';
import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language';
Expand Down Expand Up @@ -89,9 +90,10 @@ export async function* runEsBuildBuildAction(
// Log all diagnostic (error/warning/logs) messages
await logMessages(logger, result, colors, jsonLogs);
} finally {
// Ensure Sass workers are shutdown if not watching
// Ensure workers are shutdown if not watching
if (!watch) {
shutdownSassWorkerPool();
await shutdownEsbuild();
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In the non-watch path, result.dispose() is not called. While shutdownEsbuild() stops the shared esbuild process, explicitly disposing the result is consistent with the watch-mode cleanup (line 221) and ensures that any incremental build resources (like the esbuild context) are properly released. Using result?.dispose() is safer in case the build action itself failed.

Suggested change
await shutdownEsbuild();
await Promise.allSettled([result?.dispose(), shutdownEsbuild()]);

Copy link
Copy Markdown
Contributor Author

@Aukevanoost Aukevanoost May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While fair, this will throw an "used before initialized" error since technically, result isn't initialized yet

}
Comment on lines 94 to 97
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When watch is enabled, the esbuild and Sass workers are not shut down if an error occurs during the watcher setup (between lines 101 and 163). In this scenario, the first finally block skips the shutdown, and the second finally block (line 221) is never reached because the try block at line 164 is not entered. Consider restructuring the cleanup logic to ensure workers are always shut down when the builder exits, for example by using a single top-level try...finally block for the entire function.

Copy link
Copy Markdown
Contributor Author

@Aukevanoost Aukevanoost May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could fix this but it feels out-of-scope, I deliberately chose a minimum fix. This would require a bigger refactor

}

Expand Down Expand Up @@ -219,6 +221,7 @@ export async function* runEsBuildBuildAction(
await Promise.allSettled([watcher.close(), result.dispose()]);

shutdownSassWorkerPool();
await shutdownEsbuild();
}
}

Expand Down
5 changes: 5 additions & 0 deletions packages/angular/build/src/tools/esbuild/bundler-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
OutputFile,
build,
context,
stop,
} from 'esbuild';
import assert from 'node:assert';
import { builtinModules } from 'node:module';
Expand Down Expand Up @@ -57,6 +58,10 @@ function isEsBuildFailure(value: unknown): value is BuildFailure {
return !!value && typeof value === 'object' && 'errors' in value && 'warnings' in value;
}

export function shutdownEsbuild(): Promise<void> {
return stop();
}

export class BundlerContext {
#esbuildContext?: BuildContext<{ metafile: true; write: false }>;
#esbuildOptions?: BuildOptions & { metafile: true; write: false };
Expand Down
Loading