Skip to content

refactor(@angular-devkit/build-angular): always return outputWithFiles when using the application builder #27446

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions goldens/public-api/angular_devkit/build_angular/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@

import { BuilderContext } from '@angular-devkit/architect';
import { BuilderOutput } from '@angular-devkit/architect';
import { BuildOptions } from 'esbuild';
import type { ConfigOptions } from 'karma';
import { Configuration } from 'webpack';
import { DevServerBuildOutput } from '@angular-devkit/build-webpack';
import type http from 'node:http';
import { json } from '@angular-devkit/core';
import { Message } from 'esbuild';
import { Metafile } from 'esbuild';
import { Observable } from 'rxjs';
import type { OnLoadResult } from 'esbuild';
import { OutputFile } from 'esbuild';
import type { PartialMessage } from 'esbuild';
import type { Plugin as Plugin_2 } from 'esbuild';
import type ts from 'typescript';
import webpack from 'webpack';
import { WebpackLoggingCallback } from '@angular-devkit/build-webpack';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import { BuilderContext, BuilderOutput } from '@angular-devkit/architect';
import { BuilderContext } from '@angular-devkit/architect';
import { existsSync } from 'node:fs';
import path from 'node:path';
import { BuildOutputFile } from '../../tools/esbuild/bundler-context';
Expand Down Expand Up @@ -37,8 +37,7 @@ const packageWatchFiles = [
'.pnp.data.json',
];

type BuildActionOutput = (ExecutionResult['outputWithFiles'] | ExecutionResult['output']) &
BuilderOutput;
export type BuildActionOutput = ExecutionResult['outputWithFiles'];

export async function* runEsBuildBuildAction(
action: (rebuildState?: RebuildState) => Promise<ExecutionResult>,
Expand Down Expand Up @@ -223,7 +222,7 @@ export async function* runEsBuildBuildAction(

async function writeAndEmitOutput(
writeToFileSystem: boolean,
{ outputFiles, output, outputWithFiles, assetFiles }: ExecutionResult,
{ outputFiles, outputWithFiles, assetFiles }: ExecutionResult,
outputOptions: NormalizedApplicationBuildOptions['outputOptions'],
writeToFileSystemFilter: ((file: BuildOutputFile) => boolean) | undefined,
): Promise<BuildActionOutput> {
Expand All @@ -234,11 +233,9 @@ async function writeAndEmitOutput(
: outputFiles;

await writeResultFiles(outputFilesToWrite, assetFiles, outputOptions);

return output;
} else {
// Requires casting due to unneeded `JsonObject` requirement. Remove once fixed.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return outputWithFiles as any;
}

// Requires casting due to unneeded `JsonObject` requirement. Remove once fixed.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return outputWithFiles as any;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@

import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import type { Plugin } from 'esbuild';
import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
import { BuildOutputFileType } from '../../tools/esbuild/bundler-context';
import { createJsonBuildManifest } from '../../tools/esbuild/utils';
import { colors as ansiColors } from '../../utils/color';
import { purgeStaleBuildCache } from '../../utils/purge-cache';
import { assertCompatibleAngularVersion } from '../../utils/version';
import { runEsBuildBuildAction } from './build-action';
import { BuildActionOutput, runEsBuildBuildAction } from './build-action';
import { executeBuild } from './execute-build';
import {
ApplicationBuilderExtensions,
Expand All @@ -24,6 +24,8 @@ import { Schema as ApplicationBuilderOptions } from './schema';

export { ApplicationBuilderOptions };

export type ApplicationBuilderOutput = BuildActionOutput;

export async function* buildApplicationInternal(
options: ApplicationBuilderInternalOptions,
// TODO: Integrate abort signal support into builder system
Expand All @@ -44,9 +46,7 @@ export async function* buildApplicationInternal(
// Determine project name from builder context target
const projectName = target?.project;
if (!projectName) {
yield { success: false, error: `The 'application' builder requires a target to be specified.` };

return;
throw new Error(`The 'application' builder requires a target to be specified.`);
}

const normalizedOptions = await normalizeOptions(context, projectName, options, extensions);
Expand All @@ -57,21 +57,15 @@ export async function* buildApplicationInternal(
if (writeServerBundles) {
const { browser, server } = normalizedOptions.outputOptions;
if (browser === '') {
yield {
success: false,
error: `'outputPath.browser' cannot be configured to an empty string when SSR is enabled.`,
};

return;
throw new Error(
`'outputPath.browser' cannot be configured to an empty string when SSR is enabled.`,
);
}

if (browser === server) {
yield {
success: false,
error: `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value.`,
};

return;
throw new Error(
`'outputPath.browser' and 'outputPath.server' cannot be configured to the same value.`,
);
}
}

Expand Down Expand Up @@ -140,11 +134,6 @@ export async function* buildApplicationInternal(
);
}

export interface ApplicationBuilderOutput extends BuilderOutput {
outputFiles?: BuildOutputFile[];
assetFiles?: { source: string; destination: string }[];
}

/**
* Builds an application using the `application` builder with the provided
* options.
Expand Down Expand Up @@ -202,4 +191,19 @@ export function buildApplication(
return buildApplicationInternal(options, context, undefined, extensions);
}

export default createBuilder(buildApplication);
export default createBuilder(async function* (options, context) {
for await (const result of buildApplication(options, context)) {
// The builder system (architect) currently attempts to treat all results as JSON and
// attempts to validate the object with a JSON schema validator. This can lead to slow
// build completion (even after the actual build is fully complete) or crashes if the
// size and/or quantity of output files is large. Architect only requires a `success`
// property so that is all that will be passed here if the infrastructure settings have
// not been explicitly set to avoid writes. Writing is only disabled when used directly
// by the dev server which bypasses the architect behavior.
const { success } = result;

yield {
success,
};
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import { BuilderContext, createBuilder } from '@angular-devkit/architect';
import type { Plugin } from 'esbuild';
import { constants as fsConstants } from 'node:fs';
import fs from 'node:fs/promises';
Expand All @@ -15,7 +15,7 @@ import { BuildOutputFile } from '../../tools/esbuild/bundler-context';
import { BuildOutputAsset } from '../../tools/esbuild/bundler-execution-result';
import { emitFilesToDisk } from '../../tools/esbuild/utils';
import { deleteOutputDir } from '../../utils';
import { buildApplicationInternal } from '../application';
import { ApplicationBuilderOutput, buildApplicationInternal } from '../application';
import { Schema as ApplicationBuilderOptions, OutputPathClass } from '../application/schema';
import { logBuilderStatusWarnings } from './builder-status-warnings';
import { Schema as BrowserBuilderOptions } from './schema';
Expand All @@ -34,12 +34,7 @@ export async function* buildEsbuildBrowser(
write?: boolean;
},
plugins?: Plugin[],
): AsyncIterable<
BuilderOutput & {
outputFiles?: BuildOutputFile[];
assetFiles?: { source: string; destination: string }[];
}
> {
): AsyncIterable<ApplicationBuilderOutput> {
// Inform user of status of builder and options
logBuilderStatusWarnings(userOptions, context);
const normalizedOptions = normalizeOptions(userOptions);
Expand Down Expand Up @@ -103,8 +98,8 @@ function normalizeOptions(
// We write the file directly from this builder to maintain webpack output compatibility
// and not output browser files into '/browser'.
async function writeResultFiles(
outputFiles: BuildOutputFile[],
assetFiles: BuildOutputAsset[] | undefined,
outputFiles: Readonly<BuildOutputFile[]>,
assetFiles: Readonly<BuildOutputAsset[]> | undefined,
outputPath: string,
) {
const directoryExists = new Set<string>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,16 @@ export async function* serveWithVite(
if (!result.success) {
// If server is active, send an error notification
if (result.errors?.length && server) {
const { text = '', location } = result.errors[0];
hadError = true;

server.ws.send({
type: 'error',
err: {
message: result.errors[0].text,
message: text,
stack: '',
loc: result.errors[0].location,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
loc: location as any,
},
});
}
Expand Down Expand Up @@ -375,7 +378,7 @@ function handleUpdate(
function analyzeResultFiles(
normalizePath: (id: string) => string,
htmlIndexPath: string,
resultFiles: BuildOutputFile[],
resultFiles: Readonly<BuildOutputFile[]>,
generatedFiles: Map<string, OutputFileRecord>,
) {
const seen = new Set<string>(['/index.html']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ export class ExecutionResult {
this.externalMetadata = { implicitBrowser, implicitServer, explicit: explicit ?? [] };
}

get output() {
return {
success: this.errors.length === 0,
};
}

get outputWithFiles() {
get outputWithFiles(): Readonly<{
success: boolean;
outputFiles: Readonly<BuildOutputFile[]>;
assetFiles: Readonly<BuildOutputAsset[]>;
errors: Readonly<(Message | PartialMessage)[]>;
externalMetadata: Readonly<ExternalResultMetadata> | undefined;
}> {
return {
success: this.errors.length === 0,
outputFiles: this.outputFiles,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export async function writeResultFiles(

const MAX_CONCURRENT_WRITES = 64;
export async function emitFilesToDisk<T = BuildOutputAsset | BuildOutputFile>(
files: T[],
files: Readonly<T[]>,
writeFileCallback: (file: T) => Promise<void>,
): Promise<void> {
// Write files in groups of MAX_CONCURRENT_WRITES to avoid too many open files
Expand Down
Loading