Skip to content

Commit

Permalink
refactor: add explicit types for createBuilder exports
Browse files Browse the repository at this point in the history
This is necessary as with the new compilation, leveraging pnpm-linked
first party dependencies, there are cases **during migration** where
multiple locations of the same package exist. e.g.

- `@angular_devkit/architect/node_modules/@angular-devkit/core` (pnpm package output)
- `@angular_devkit/core` (plain js sources)

This is fine, and there is no risk of wrong types, and this should only
occur during migration anyway, but it causes a few type issues as
TypeScript is unable to emit proper types for inference. This is okay,
as we'd likely even want to make use of `isolatedDeclarations` at some
point, but we just add explicit types right now.

(also making `Builder` type more type safe, checking assignability
properly).
  • Loading branch information
devversion committed Jan 15, 2025
1 parent 62e90d0 commit bc21d81
Show file tree
Hide file tree
Showing 12 changed files with 73 additions and 25 deletions.
12 changes: 12 additions & 0 deletions goldens/public-api/angular_devkit/architect/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ export class Architect {
scheduleTarget(target: Target, overrides?: json.JsonObject, scheduleOptions?: ScheduleOptions): Promise<BuilderRun>;
}

// @public
export interface Builder<OptionT extends json.JsonObject = json.JsonObject> {
// (undocumented)
[BuilderSymbol]: true;
// (undocumented)
[BuilderVersionSymbol]: string;
// (undocumented)
__OptionT: OptionT;
// (undocumented)
handler: JobHandler<json.JsonObject, BuilderInput, BuilderOutput>;
}

// @public
export interface BuilderContext {
addTeardown(teardown: () => Promise<void> | void): void;
Expand Down
1 change: 1 addition & 0 deletions packages/angular/build/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ ts_project(
"//packages/angular/ssr:ssr_rjs",
"//packages/angular/ssr/node:node_rjs",
"//packages/angular_devkit/architect:architect_rjs",
"//packages/angular_devkit/core:core_rjs",
],
)

Expand Down
8 changes: 6 additions & 2 deletions packages/angular/build/src/builders/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import { Builder, BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import { json } from '@angular-devkit/core';
import assert from 'node:assert';
import fs from 'node:fs/promises';
import path from 'node:path';
Expand Down Expand Up @@ -259,4 +260,7 @@ function generateFullPath(
return fullFilePath;
}

export default createBuilder(buildApplication);
const builder: Builder<ApplicationBuilderOptions & json.JsonObject> =
createBuilder(buildApplication);

export default builder;
10 changes: 8 additions & 2 deletions packages/angular/build/src/builders/dev-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { createBuilder } from '@angular-devkit/architect';
import { Builder, createBuilder } from '@angular-devkit/architect';
import { json } from '@angular-devkit/core';
import { execute } from './builder';
import type { DevServerBuilderOutput } from './output';
import type { Schema as DevServerBuilderOptions } from './schema';
Expand All @@ -16,7 +17,12 @@ export {
type DevServerBuilderOutput,
execute as executeDevServerBuilder,
};
export default createBuilder<DevServerBuilderOptions, DevServerBuilderOutput>(execute);
const builder: Builder<DevServerBuilderOptions & json.JsonObject> = createBuilder<
DevServerBuilderOptions,
DevServerBuilderOutput
>(execute);

export default builder;

// Temporary export to support specs
export { execute as executeDevServer };
9 changes: 7 additions & 2 deletions packages/angular/build/src/builders/extract-i18n/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { createBuilder } from '@angular-devkit/architect';
import { Builder, createBuilder } from '@angular-devkit/architect';
import { json } from '@angular-devkit/core';
import { execute } from './builder';
import type { Schema as ExtractI18nBuilderOptions } from './schema';

export { ExtractI18nBuilderOptions, execute };
export default createBuilder<ExtractI18nBuilderOptions>(execute);

const builder: Builder<ExtractI18nBuilderOptions & json.JsonObject> =
createBuilder<ExtractI18nBuilderOptions>(execute);

export default builder;
9 changes: 7 additions & 2 deletions packages/angular/build/src/builders/ng-packagr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { createBuilder } from '@angular-devkit/architect';
import { Builder, createBuilder } from '@angular-devkit/architect';
import { json } from '@angular-devkit/core';
import { execute } from './builder';
import type { Schema as NgPackagrBuilderOptions } from './schema';

export { type NgPackagrBuilderOptions, execute };
export default createBuilder<NgPackagrBuilderOptions>(execute);

const builder: Builder<NgPackagrBuilderOptions & json.JsonObject> =
createBuilder<NgPackagrBuilderOptions>(execute);

export default builder;
5 changes: 5 additions & 0 deletions packages/angular_devkit/architect/src/create-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import { Builder, BuilderSymbol, BuilderVersionSymbol } from './internal';
import { JobInboundMessageKind, createJobHandler } from './jobs';
import { scheduleByName, scheduleByTarget } from './schedule-by-name';

export type { Builder };

// eslint-disable-next-line max-lines-per-function
export function createBuilder<OptT = json.JsonObject, OutT extends BuilderOutput = BuilderOutput>(
fn: BuilderHandlerFn<OptT>,
Expand Down Expand Up @@ -252,6 +254,9 @@ export function createBuilder<OptT = json.JsonObject, OutT extends BuilderOutput
handler,
[BuilderSymbol]: true,
[BuilderVersionSymbol]: require('../package.json').version,
// Only needed for type safety around `Builder` types.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
__OptionT: null!,
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/angular_devkit/architect/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ import * as jobs from './jobs';

export * from './api';
export { Architect, type ScheduleOptions } from './architect';
export { createBuilder } from './create-builder';
export { createBuilder, type Builder } from './create-builder';

export { jobs };
1 change: 1 addition & 0 deletions packages/angular_devkit/architect/src/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export interface Builder<OptionT extends json.JsonObject = json.JsonObject> {
// Metadata associated with this builder.
[BuilderSymbol]: true;
[BuilderVersionSymbol]: string;
__OptionT: OptionT;
}

export interface ArchitectHost<BuilderInfoT extends BuilderInfo = BuilderInfo> {
Expand Down
1 change: 1 addition & 0 deletions packages/angular_devkit/build_webpack/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ ts_project(
"//:node_modules/webpack",
"//:node_modules/webpack-dev-server",
"//packages/angular_devkit/architect:architect_rjs",
"//packages/angular_devkit/core:core_rjs",
],
)

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

import { BuilderContext, createBuilder } from '@angular-devkit/architect';
import { Builder, BuilderContext, createBuilder } from '@angular-devkit/architect';
import { json } from '@angular-devkit/core';
import { resolve as pathResolve } from 'path';
import { Observable, from, isObservable, of, switchMap } from 'rxjs';
import webpack from 'webpack';
Expand Down Expand Up @@ -124,12 +125,15 @@ export function runWebpackDevServer(
);
}

export default createBuilder<WebpackDevServerBuilderSchema, DevServerBuildOutput>(
(options, context) => {
const configPath = pathResolve(context.workspaceRoot, options.webpackConfig);
const builder: Builder<WebpackDevServerBuilderSchema & json.JsonObject> = createBuilder<
WebpackDevServerBuilderSchema,
DevServerBuildOutput
>((options, context) => {
const configPath = pathResolve(context.workspaceRoot, options.webpackConfig);

return from(getWebpackConfig(configPath)).pipe(
switchMap((config) => runWebpackDevServer(config, context)),
);
},
);
return from(getWebpackConfig(configPath)).pipe(
switchMap((config) => runWebpackDevServer(config, context)),
);
});

export default builder;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import { Builder, BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import { json } from '@angular-devkit/core';
import { resolve as pathResolve } from 'path';
import { Observable, from, isObservable, of, switchMap } from 'rxjs';
import webpack from 'webpack';
Expand Down Expand Up @@ -120,10 +121,13 @@ export function runWebpack(
);
}

export default createBuilder<WebpackBuilderSchema>((options, context) => {
const configPath = pathResolve(context.workspaceRoot, options.webpackConfig);
const builder: Builder<WebpackBuilderSchema & json.JsonObject> =
createBuilder<WebpackBuilderSchema>((options, context) => {
const configPath = pathResolve(context.workspaceRoot, options.webpackConfig);

return from(getWebpackConfig(configPath)).pipe(
switchMap((config) => runWebpack(config, context)),
);
});
return from(getWebpackConfig(configPath)).pipe(
switchMap((config) => runWebpack(config, context)),
);
});

export default builder;

0 comments on commit bc21d81

Please sign in to comment.