Skip to content

Commit 9e1d328

Browse files
committed
refactor(@angular/build): clean up duplicate of code in the karma builders
This ensures a cleaner and easier to follow code and also avoid having the fix bugs in multiple places.
1 parent 9c5d75d commit 9e1d328

File tree

6 files changed

+157
-153
lines changed

6 files changed

+157
-153
lines changed

goldens/public-api/angular/build/index.api.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,7 @@ export function executeDevServerBuilder(options: DevServerBuilderOptions, contex
153153
export function executeExtractI18nBuilder(options: ExtractI18nBuilderOptions, context: BuilderContext, extensions?: ApplicationBuilderExtensions): Promise<BuilderOutput>;
154154

155155
// @public
156-
export function executeKarmaBuilder(options: KarmaBuilderOptions, context: BuilderContext, transforms?: {
157-
karmaOptions?: (options: KarmaConfigOptions) => KarmaConfigOptions;
158-
}): AsyncIterable<BuilderOutput>;
156+
export function executeKarmaBuilder(options: KarmaBuilderOptions, context: BuilderContext, transforms?: KarmaBuilderTransformsOptions): AsyncIterable<BuilderOutput>;
159157

160158
// @public
161159
export function executeNgPackagrBuilder(options: NgPackagrBuilderOptions, context: BuilderContext): AsyncIterableIterator<BuilderOutput>;

packages/angular/build/src/builders/karma/application_builder.ts

Lines changed: 96 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { randomUUID } from 'node:crypto';
1212
import * as fs from 'node:fs/promises';
1313
import type { IncomingMessage, ServerResponse } from 'node:http';
1414
import { createRequire } from 'node:module';
15-
import * as path from 'node:path';
15+
import path from 'node:path';
1616
import { ReadableStreamController } from 'node:stream/web';
1717
import { globSync } from 'tinyglobby';
1818
import { BuildOutputFileType } from '../../tools/esbuild/bundler-context';
@@ -24,7 +24,9 @@ import { ApplicationBuilderInternalOptions } from '../application/options';
2424
import { Result, ResultFile, ResultKind } from '../application/results';
2525
import { OutputHashing } from '../application/schema';
2626
import { findTests, getTestEntrypoints } from './find-tests';
27-
import { NormalizedKarmaBuilderOptions } from './options';
27+
import { NormalizedKarmaBuilderOptions, normalizeOptions } from './options';
28+
import { Schema as KarmaBuilderOptions } from './schema';
29+
import type { KarmaBuilderTransformsOptions } from './index';
2830

2931
const localResolve = createRequire(__filename).resolve;
3032
const isWindows = process.platform === 'win32';
@@ -275,21 +277,20 @@ function injectKarmaReporter(
275277
}
276278

277279
export function execute(
278-
options: NormalizedKarmaBuilderOptions,
280+
options: KarmaBuilderOptions,
279281
context: BuilderContext,
280-
karmaOptions: ConfigOptions,
281-
transforms: {
282-
// The karma options transform cannot be async without a refactor of the builder implementation
283-
karmaOptions?: (options: ConfigOptions) => ConfigOptions;
284-
} = {},
282+
transforms?: KarmaBuilderTransformsOptions,
285283
): AsyncIterable<BuilderOutput> {
284+
const normalizedOptions = normalizeOptions(context, options);
285+
const karmaOptions = getBaseKarmaOptions(normalizedOptions, context);
286+
286287
let karmaServer: Server;
287288

288289
return new ReadableStream({
289290
async start(controller) {
290291
let init;
291292
try {
292-
init = await initializeApplication(options, context, karmaOptions, transforms);
293+
init = await initializeApplication(normalizedOptions, context, karmaOptions, transforms);
293294
} catch (err) {
294295
if (err instanceof ApplicationBuildError) {
295296
controller.enqueue({ success: false, message: err.message });
@@ -336,13 +337,9 @@ async function getProjectSourceRoot(context: BuilderContext): Promise<string> {
336337
return projectSourceRoot;
337338
}
338339

339-
function normalizePolyfills(polyfills: string | string[] | undefined): [string[], string[]] {
340-
if (typeof polyfills === 'string') {
341-
polyfills = [polyfills];
342-
} else if (!polyfills) {
343-
polyfills = [];
344-
}
345-
340+
function normalizePolyfills(
341+
polyfills: string[] = [],
342+
): [polyfills: string[], jasmineCleanup: string[]] {
346343
const jasmineGlobalEntryPoint = localResolve('./polyfills/jasmine_global.js');
347344
const jasmineGlobalCleanupEntrypoint = localResolve('./polyfills/jasmine_global_cleanup.js');
348345
const sourcemapEntrypoint = localResolve('./polyfills/init_sourcemaps.js');
@@ -379,9 +376,7 @@ async function initializeApplication(
379376
options: NormalizedKarmaBuilderOptions,
380377
context: BuilderContext,
381378
karmaOptions: ConfigOptions,
382-
transforms: {
383-
karmaOptions?: (options: ConfigOptions) => ConfigOptions;
384-
} = {},
379+
transforms?: KarmaBuilderTransformsOptions,
385380
): Promise<
386381
[typeof import('karma'), Config & ConfigOptions, BuildOptions, AsyncIterator<Result> | null]
387382
> {
@@ -423,13 +418,7 @@ async function initializeApplication(
423418
index: false,
424419
outputHashing: OutputHashing.None,
425420
optimization: false,
426-
sourceMap: options.codeCoverage
427-
? {
428-
scripts: true,
429-
styles: true,
430-
vendor: true,
431-
}
432-
: options.sourceMap,
421+
sourceMap: options.sourceMap,
433422
instrumentForCoverage,
434423
styles: options.styles,
435424
scripts: options.scripts,
@@ -551,8 +540,8 @@ async function initializeApplication(
551540
}
552541

553542
const parsedKarmaConfig: Config & ConfigOptions = await karma.config.parseConfig(
554-
options.karmaConfig && path.resolve(context.workspaceRoot, options.karmaConfig),
555-
transforms.karmaOptions ? transforms.karmaOptions(karmaOptions) : karmaOptions,
543+
options.karmaConfig,
544+
transforms?.karmaOptions ? await transforms.karmaOptions(karmaOptions) : karmaOptions,
556545
{ promiseConfig: true, throwErrors: true },
557546
);
558547

@@ -718,3 +707,82 @@ function getInstrumentationExcludedPaths(root: string, excludedPaths: string[]):
718707

719708
return excluded;
720709
}
710+
function getBaseKarmaOptions(
711+
options: NormalizedKarmaBuilderOptions,
712+
context: BuilderContext,
713+
): ConfigOptions {
714+
// Determine project name from builder context target
715+
const projectName = context.target?.project;
716+
if (!projectName) {
717+
throw new Error(`The 'karma' builder requires a target to be specified.`);
718+
}
719+
720+
const karmaOptions: ConfigOptions = options.karmaConfig
721+
? {}
722+
: getBuiltInKarmaConfig(context.workspaceRoot, projectName);
723+
724+
const singleRun = !options.watch;
725+
karmaOptions.singleRun = singleRun;
726+
727+
// Workaround https://github.com/angular/angular-cli/issues/28271, by clearing context by default
728+
// for single run executions. Not clearing context for multi-run (watched) builds allows the
729+
// Jasmine Spec Runner to be visible in the browser after test execution.
730+
karmaOptions.client ??= {};
731+
karmaOptions.client.clearContext ??= singleRun;
732+
733+
// Convert browsers from a string to an array
734+
if (options.browsers) {
735+
karmaOptions.browsers = options.browsers;
736+
}
737+
738+
if (options.reporters) {
739+
karmaOptions.reporters = options.reporters;
740+
}
741+
742+
return karmaOptions;
743+
}
744+
745+
function getBuiltInKarmaConfig(
746+
workspaceRoot: string,
747+
projectName: string,
748+
): ConfigOptions & Record<string, unknown> {
749+
let coverageFolderName = projectName.charAt(0) === '@' ? projectName.slice(1) : projectName;
750+
coverageFolderName = coverageFolderName.toLowerCase();
751+
752+
const workspaceRootRequire = createRequire(workspaceRoot + '/');
753+
754+
// Any changes to the config here need to be synced to: packages/schematics/angular/config/files/karma.conf.js.template
755+
return {
756+
basePath: '',
757+
frameworks: ['jasmine'],
758+
plugins: [
759+
'karma-jasmine',
760+
'karma-chrome-launcher',
761+
'karma-jasmine-html-reporter',
762+
'karma-coverage',
763+
].map((p) => workspaceRootRequire(p)),
764+
jasmineHtmlReporter: {
765+
suppressAll: true, // removes the duplicated traces
766+
},
767+
coverageReporter: {
768+
dir: path.join(workspaceRoot, 'coverage', coverageFolderName),
769+
subdir: '.',
770+
reporters: [{ type: 'html' }, { type: 'text-summary' }],
771+
},
772+
reporters: ['progress', 'kjhtml'],
773+
browsers: ['Chrome'],
774+
customLaunchers: {
775+
// Chrome configured to run in a bazel sandbox.
776+
// Disable the use of the gpu and `/dev/shm` because it causes Chrome to
777+
// crash on some environments.
778+
// See:
779+
// https://github.com/puppeteer/puppeteer/blob/v1.0.0/docs/troubleshooting.md#tips
780+
// https://stackoverflow.com/questions/50642308/webdriverexception-unknown-error-devtoolsactiveport-file-doesnt-exist-while-t
781+
ChromeHeadlessNoSandbox: {
782+
base: 'ChromeHeadless',
783+
flags: ['--no-sandbox', '--headless', '--disable-gpu', '--disable-dev-shm-usage'],
784+
},
785+
},
786+
restartOnFileChange: true,
787+
};
788+
}

packages/angular/build/src/builders/karma/index.ts

Lines changed: 6 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -13,114 +13,24 @@ import {
1313
createBuilder,
1414
} from '@angular-devkit/architect';
1515
import type { ConfigOptions } from 'karma';
16-
import { createRequire } from 'node:module';
17-
import path from 'node:path';
18-
import { NormalizedKarmaBuilderOptions, normalizeOptions } from './options';
16+
1917
import type { Schema as KarmaBuilderOptions } from './schema';
2018

21-
export type KarmaConfigOptions = ConfigOptions & {
22-
buildWebpack?: unknown;
23-
configFile?: string;
24-
};
19+
export interface KarmaBuilderTransformsOptions {
20+
karmaOptions?: (options: ConfigOptions) => ConfigOptions | Promise<ConfigOptions>;
21+
}
2522

2623
/**
2724
* @experimental Direct usage of this function is considered experimental.
2825
*/
2926
export async function* execute(
3027
options: KarmaBuilderOptions,
3128
context: BuilderContext,
32-
transforms: {
33-
// The karma options transform cannot be async without a refactor of the builder implementation
34-
karmaOptions?: (options: KarmaConfigOptions) => KarmaConfigOptions;
35-
} = {},
29+
transforms?: KarmaBuilderTransformsOptions,
3630
): AsyncIterable<BuilderOutput> {
3731
const { execute } = await import('./application_builder');
38-
const normalizedOptions = normalizeOptions(options);
39-
const karmaOptions = getBaseKarmaOptions(normalizedOptions, context);
40-
41-
yield* execute(normalizedOptions, context, karmaOptions, transforms);
42-
}
43-
44-
function getBaseKarmaOptions(
45-
options: NormalizedKarmaBuilderOptions,
46-
context: BuilderContext,
47-
): KarmaConfigOptions {
48-
const singleRun = !options.watch;
49-
50-
// Determine project name from builder context target
51-
const projectName = context.target?.project;
52-
if (!projectName) {
53-
throw new Error(`The 'karma' builder requires a target to be specified.`);
54-
}
55-
56-
const karmaOptions: KarmaConfigOptions = options.karmaConfig
57-
? {}
58-
: getBuiltInKarmaConfig(context.workspaceRoot, projectName);
59-
60-
karmaOptions.singleRun = singleRun;
61-
62-
// Workaround https://github.com/angular/angular-cli/issues/28271, by clearing context by default
63-
// for single run executions. Not clearing context for multi-run (watched) builds allows the
64-
// Jasmine Spec Runner to be visible in the browser after test execution.
65-
karmaOptions.client ??= {};
66-
karmaOptions.client.clearContext ??= singleRun ?? false; // `singleRun` defaults to `false` per Karma docs.
67-
68-
// Convert browsers from a string to an array
69-
if (options.browsers) {
70-
karmaOptions.browsers = options.browsers;
71-
}
72-
73-
if (options.reporters) {
74-
karmaOptions.reporters = options.reporters;
75-
}
76-
77-
return karmaOptions;
78-
}
79-
80-
function getBuiltInKarmaConfig(
81-
workspaceRoot: string,
82-
projectName: string,
83-
): ConfigOptions & Record<string, unknown> {
84-
let coverageFolderName = projectName.charAt(0) === '@' ? projectName.slice(1) : projectName;
85-
coverageFolderName = coverageFolderName.toLowerCase();
86-
87-
const workspaceRootRequire = createRequire(workspaceRoot + '/');
8832

89-
// Any changes to the config here need to be synced to: packages/schematics/angular/config/files/karma.conf.js.template
90-
return {
91-
basePath: '',
92-
rootUrl: '/',
93-
frameworks: ['jasmine'],
94-
plugins: [
95-
'karma-jasmine',
96-
'karma-chrome-launcher',
97-
'karma-jasmine-html-reporter',
98-
'karma-coverage',
99-
].map((p) => workspaceRootRequire(p)),
100-
jasmineHtmlReporter: {
101-
suppressAll: true, // removes the duplicated traces
102-
},
103-
coverageReporter: {
104-
dir: path.join(workspaceRoot, 'coverage', coverageFolderName),
105-
subdir: '.',
106-
reporters: [{ type: 'html' }, { type: 'text-summary' }],
107-
},
108-
reporters: ['progress', 'kjhtml'],
109-
browsers: ['Chrome'],
110-
customLaunchers: {
111-
// Chrome configured to run in a bazel sandbox.
112-
// Disable the use of the gpu and `/dev/shm` because it causes Chrome to
113-
// crash on some environments.
114-
// See:
115-
// https://github.com/puppeteer/puppeteer/blob/v1.0.0/docs/troubleshooting.md#tips
116-
// https://stackoverflow.com/questions/50642308/webdriverexception-unknown-error-devtoolsactiveport-file-doesnt-exist-while-t
117-
ChromeHeadlessNoSandbox: {
118-
base: 'ChromeHeadless',
119-
flags: ['--no-sandbox', '--headless', '--disable-gpu', '--disable-dev-shm-usage'],
120-
},
121-
},
122-
restartOnFileChange: true,
123-
};
33+
yield* execute(options, context, transforms);
12434
}
12535

12636
export type { KarmaBuilderOptions };

packages/angular/build/src/builders/karma/options.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import type { BuilderContext } from '@angular-devkit/architect';
10+
import { resolve } from 'node:path';
911
import { Schema as KarmaBuilderOptions } from './schema';
1012

11-
export type NormalizedKarmaBuilderOptions = Awaited<ReturnType<typeof normalizeOptions>>;
13+
export type NormalizedKarmaBuilderOptions = ReturnType<typeof normalizeOptions>;
1214

13-
export function normalizeOptions(options: KarmaBuilderOptions) {
14-
const { watch = true, include = [], exclude = [], reporters = [], browsers, ...rest } = options;
15+
export function normalizeOptions(context: BuilderContext, options: KarmaBuilderOptions) {
16+
const {
17+
sourceMap,
18+
karmaConfig,
19+
browsers,
20+
watch = true,
21+
include = [],
22+
exclude = [],
23+
reporters = [],
24+
...rest
25+
} = options;
1526

1627
let normalizedBrowsers: string[] | undefined;
1728
if (typeof options.browsers === 'string' && options.browsers) {
@@ -25,12 +36,23 @@ export function normalizeOptions(options: KarmaBuilderOptions) {
2536
.reduce<string[]>((acc, curr) => acc.concat(curr.split(',')), [])
2637
.filter((x) => !!x);
2738

39+
// Sourcemaps are always needed when code coverage is enabled.
40+
const normalizedSourceMap = options.codeCoverage
41+
? {
42+
scripts: true,
43+
styles: true,
44+
vendor: true,
45+
}
46+
: sourceMap;
47+
2848
return {
49+
...rest,
50+
sourceMap: normalizedSourceMap,
51+
karmaConfig: karmaConfig ? resolve(context.workspaceRoot, karmaConfig) : undefined,
2952
reporters: normalizedReporters.length ? normalizedReporters : undefined,
3053
browsers: normalizedBrowsers,
3154
watch,
3255
include,
3356
exclude,
34-
...rest,
3557
};
3658
}

packages/angular/build/src/private.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ export { buildApplicationInternal } from './builders/application';
2626
export type { ApplicationBuilderInternalOptions } from './builders/application/options';
2727
export { type Result, type ResultFile, ResultKind } from './builders/application/results';
2828
export { serveWithVite } from './builders/dev-server/vite-server';
29-
export { execute as executeKarmaInternal } from './builders/karma/application_builder';
3029

3130
// Tools
3231
export * from './tools/babel/plugins';

0 commit comments

Comments
 (0)