From be08d1e3ebc4b321cbacd513f68f91ca450adbd8 Mon Sep 17 00:00:00 2001 From: David Michon Date: Mon, 29 Sep 2025 22:51:19 +0000 Subject: [PATCH] [heft] Revert task/phase start/finish hooks to be synchronous --- apps/heft/src/cli/HeftActionRunner.ts | 24 +++++++++---------- .../heft/src/pluginFramework/HeftLifecycle.ts | 10 ++++---- .../pluginFramework/HeftLifecycleSession.ts | 18 +++++++------- .../src/index.ts | 8 +++---- .../heft/sync-hooks_2025-09-29-22-50.json | 10 ++++++++ .../sync-hooks_2025-09-29-22-50.json | 10 ++++++++ common/reviews/api/heft.api.md | 9 +++---- common/reviews/api/operation-graph.api.md | 12 +++++----- libraries/operation-graph/src/Operation.ts | 8 +++---- .../src/OperationExecutionManager.ts | 24 +++++++++---------- 10 files changed, 76 insertions(+), 57 deletions(-) create mode 100644 common/changes/@rushstack/heft/sync-hooks_2025-09-29-22-50.json create mode 100644 common/changes/@rushstack/operation-graph/sync-hooks_2025-09-29-22-50.json diff --git a/apps/heft/src/cli/HeftActionRunner.ts b/apps/heft/src/cli/HeftActionRunner.ts index bf3d37f2664..d0d60d26553 100644 --- a/apps/heft/src/cli/HeftActionRunner.ts +++ b/apps/heft/src/cli/HeftActionRunner.ts @@ -387,32 +387,32 @@ export class HeftActionRunner { parallelism: this._parallelism, abortSignal, requestRun, - beforeExecuteOperationAsync: async ( + beforeExecuteOperation( operation: Operation - ) => { + ): void { if (taskStart.isUsed()) { - await taskStart.promise({ operation }); + taskStart.call({ operation }); } }, - afterExecuteOperationAsync: async ( + afterExecuteOperation( operation: Operation - ) => { + ): void { if (taskFinish.isUsed()) { - await taskFinish.promise({ operation }); + taskFinish.call({ operation }); } }, - beforeExecuteOperationGroupAsync: async ( + beforeExecuteOperationGroup( operationGroup: OperationGroupRecord - ) => { + ): void { if (operationGroup.metadata.phase && phaseStart.isUsed()) { - await phaseStart.promise({ operation: operationGroup }); + phaseStart.call({ operation: operationGroup }); } }, - afterExecuteOperationGroupAsync: async ( + afterExecuteOperationGroup( operationGroup: OperationGroupRecord - ) => { + ): void { if (operationGroup.metadata.phase && phaseFinish.isUsed()) { - await phaseFinish.promise({ operation: operationGroup }); + phaseFinish.call({ operation: operationGroup }); } } }; diff --git a/apps/heft/src/pluginFramework/HeftLifecycle.ts b/apps/heft/src/pluginFramework/HeftLifecycle.ts index 97a1c15f8eb..29f598c1258 100644 --- a/apps/heft/src/pluginFramework/HeftLifecycle.ts +++ b/apps/heft/src/pluginFramework/HeftLifecycle.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { AsyncParallelHook } from 'tapable'; +import { AsyncParallelHook, SyncHook } from 'tapable'; import { InternalError } from '@rushstack/node-core-library'; import { HeftPluginConfiguration } from '../configuration/HeftPluginConfiguration'; @@ -72,10 +72,10 @@ export class HeftLifecycle extends HeftPluginHost { toolStart: new AsyncParallelHook(), toolFinish: new AsyncParallelHook(), recordMetrics: internalHeftSession.metricsCollector.recordMetricsHook, - taskStart: new AsyncParallelHook(['task']), - taskFinish: new AsyncParallelHook(['task']), - phaseStart: new AsyncParallelHook(['phase']), - phaseFinish: new AsyncParallelHook(['phase']) + taskStart: new SyncHook(['task']), + taskFinish: new SyncHook(['task']), + phaseStart: new SyncHook(['phase']), + phaseFinish: new SyncHook(['phase']) }; } diff --git a/apps/heft/src/pluginFramework/HeftLifecycleSession.ts b/apps/heft/src/pluginFramework/HeftLifecycleSession.ts index 8cadcd7c534..b881f0cd742 100644 --- a/apps/heft/src/pluginFramework/HeftLifecycleSession.ts +++ b/apps/heft/src/pluginFramework/HeftLifecycleSession.ts @@ -2,7 +2,7 @@ // See LICENSE in the project root for license information. import * as path from 'path'; -import type { AsyncParallelHook } from 'tapable'; +import type { AsyncParallelHook, SyncHook } from 'tapable'; import type { IHeftRecordMetricsHookOptions, MetricsCollector } from '../metrics/MetricsCollector'; import type { ScopedLogger, IScopedLogger } from './logging/ScopedLogger'; @@ -144,35 +144,35 @@ export interface IHeftLifecycleHooks { /** * The `taskStart` hook is called at the beginning of a task. It is called before the task has begun - * to execute. To use it, call `taskStart.tapPromise(, )`. + * to execute. To use it, call `taskStart.tap(, )`. * * @public */ - taskStart: AsyncParallelHook; + taskStart: SyncHook; /** * The `taskFinish` hook is called at the end of a task. It is called after the task has completed - * execution. To use it, call `taskFinish.tapPromise(, )`. + * execution. To use it, call `taskFinish.tap(, )`. * * @public */ - taskFinish: AsyncParallelHook; + taskFinish: SyncHook; /** * The `phaseStart` hook is called at the beginning of a phase. It is called before the phase has - * begun to execute. To use it, call `phaseStart.tapPromise(, )`. + * begun to execute. To use it, call `phaseStart.tap(, )`. * * @public */ - phaseStart: AsyncParallelHook; + phaseStart: SyncHook; /** * The `phaseFinish` hook is called at the end of a phase. It is called after the phase has completed - * execution. To use it, call `phaseFinish.tapPromise(, )`. + * execution. To use it, call `phaseFinish.tap(, )`. * * @public */ - phaseFinish: AsyncParallelHook; + phaseFinish: SyncHook; } /** diff --git a/build-tests/heft-example-lifecycle-plugin/src/index.ts b/build-tests/heft-example-lifecycle-plugin/src/index.ts index 993db618ad4..b1ed00244cd 100644 --- a/build-tests/heft-example-lifecycle-plugin/src/index.ts +++ b/build-tests/heft-example-lifecycle-plugin/src/index.ts @@ -15,7 +15,7 @@ export const PLUGIN_NAME: 'example-lifecycle-plugin' = 'example-lifecycle-plugin export default class ExampleLifecyclePlugin implements IHeftLifecyclePlugin { public apply(session: IHeftLifecycleSession): void { const { logger } = session; - session.hooks.taskFinish.tapPromise(PLUGIN_NAME, async (options: IHeftTaskFinishHookOptions) => { + session.hooks.taskFinish.tap(PLUGIN_NAME, (options: IHeftTaskFinishHookOptions) => { const { operation: { metadata: { task }, @@ -29,7 +29,7 @@ export default class ExampleLifecyclePlugin implements IHeftLifecyclePlugin { } }); - session.hooks.taskStart.tapPromise(PLUGIN_NAME, async (options: IHeftTaskStartHookOptions) => { + session.hooks.taskStart.tap(PLUGIN_NAME, (options: IHeftTaskStartHookOptions) => { const { operation: { metadata: { task } @@ -38,7 +38,7 @@ export default class ExampleLifecyclePlugin implements IHeftLifecyclePlugin { logger.terminal.writeLine(`--- ${task.taskName} started ---`); }); - session.hooks.phaseStart.tapPromise(PLUGIN_NAME, async (options: IHeftPhaseStartHookOptions) => { + session.hooks.phaseStart.tap(PLUGIN_NAME, (options: IHeftPhaseStartHookOptions) => { const { operation: { metadata: { phase } @@ -47,7 +47,7 @@ export default class ExampleLifecyclePlugin implements IHeftLifecyclePlugin { logger.terminal.writeLine(`--- ${phase.phaseName} started ---`); }); - session.hooks.phaseFinish.tapPromise(PLUGIN_NAME, async (options: IHeftPhaseFinishHookOptions) => { + session.hooks.phaseFinish.tap(PLUGIN_NAME, (options: IHeftPhaseFinishHookOptions) => { const { operation: { metadata: { phase }, diff --git a/common/changes/@rushstack/heft/sync-hooks_2025-09-29-22-50.json b/common/changes/@rushstack/heft/sync-hooks_2025-09-29-22-50.json new file mode 100644 index 00000000000..9bc8ff5ca98 --- /dev/null +++ b/common/changes/@rushstack/heft/sync-hooks_2025-09-29-22-50.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft", + "comment": "(BREAKING CHANGE) Make the `taskStart`/`taskFinish`/`phaseStart`/`phaseFinish` hooks synchronous to signify that they are not intended to be used for expensive work.", + "type": "minor" + } + ], + "packageName": "@rushstack/heft" +} \ No newline at end of file diff --git a/common/changes/@rushstack/operation-graph/sync-hooks_2025-09-29-22-50.json b/common/changes/@rushstack/operation-graph/sync-hooks_2025-09-29-22-50.json new file mode 100644 index 00000000000..775d0d19b78 --- /dev/null +++ b/common/changes/@rushstack/operation-graph/sync-hooks_2025-09-29-22-50.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/operation-graph", + "comment": "(BREAKING CHANGE) Revert the extensibility points for `(before/after)ExecuteOperation(Group)?Async` to be synchronous to signify that they are only meant for logging, not for expensive work.", + "type": "minor" + } + ], + "packageName": "@rushstack/operation-graph" +} \ No newline at end of file diff --git a/common/reviews/api/heft.api.md b/common/reviews/api/heft.api.md index caab5d5e14f..7c13f35565a 100644 --- a/common/reviews/api/heft.api.md +++ b/common/reviews/api/heft.api.md @@ -38,6 +38,7 @@ import type { Operation } from '@rushstack/operation-graph'; import type { OperationGroupRecord } from '@rushstack/operation-graph'; import { PathResolutionMethod } from '@rushstack/heft-config-file'; import { PropertyInheritanceCustomFunction } from '@rushstack/heft-config-file'; +import type { SyncHook } from 'tapable'; export { CommandLineChoiceListParameter } @@ -152,11 +153,11 @@ export interface IHeftLifecycleCleanHookOptions { // @public export interface IHeftLifecycleHooks { clean: AsyncParallelHook; - phaseFinish: AsyncParallelHook; - phaseStart: AsyncParallelHook; + phaseFinish: SyncHook; + phaseStart: SyncHook; recordMetrics: AsyncParallelHook; - taskFinish: AsyncParallelHook; - taskStart: AsyncParallelHook; + taskFinish: SyncHook; + taskStart: SyncHook; toolFinish: AsyncParallelHook; toolStart: AsyncParallelHook; } diff --git a/common/reviews/api/operation-graph.api.md b/common/reviews/api/operation-graph.api.md index eac2ff25c29..261057fb62a 100644 --- a/common/reviews/api/operation-graph.api.md +++ b/common/reviews/api/operation-graph.api.md @@ -30,8 +30,8 @@ export interface ICancelCommandMessage { // @beta export interface IExecuteOperationContext extends Omit { - afterExecuteAsync(operation: Operation, state: IOperationState): Promise; - beforeExecuteAsync(operation: Operation, state: IOperationState): Promise; + afterExecute(operation: Operation, state: IOperationState): void; + beforeExecute(operation: Operation, state: IOperationState): void; queueWork(workFn: () => Promise, priority: number): Promise; requestRun?: OperationRequestRunCallback; terminal: ITerminal; @@ -48,13 +48,13 @@ export interface IOperationExecutionOptions) => Promise; + afterExecuteOperation?: (operation: Operation) => void; // (undocumented) - afterExecuteOperationGroupAsync?: (operationGroup: OperationGroupRecord) => Promise; + afterExecuteOperationGroup?: (operationGroup: OperationGroupRecord) => void; // (undocumented) - beforeExecuteOperationAsync?: (operation: Operation) => Promise; + beforeExecuteOperation?: (operation: Operation) => void; // (undocumented) - beforeExecuteOperationGroupAsync?: (operationGroup: OperationGroupRecord) => Promise; + beforeExecuteOperationGroup?: (operationGroup: OperationGroupRecord) => void; // (undocumented) parallelism: number; // (undocumented) diff --git a/libraries/operation-graph/src/Operation.ts b/libraries/operation-graph/src/Operation.ts index 75528ce1295..71ffa0887c1 100644 --- a/libraries/operation-graph/src/Operation.ts +++ b/libraries/operation-graph/src/Operation.ts @@ -62,12 +62,12 @@ export interface IExecuteOperationContext extends Omit; + beforeExecute(operation: Operation, state: IOperationState): void; /** * Function to invoke after execution of an operation, for logging. */ - afterExecuteAsync(operation: Operation, state: IOperationState): Promise; + afterExecute(operation: Operation, state: IOperationState): void; /** * Function used to schedule the concurrency-limited execution of an operation. @@ -328,7 +328,7 @@ export class Operation) => Promise; - afterExecuteOperationAsync?: (operation: Operation) => Promise; - beforeExecuteOperationGroupAsync?: (operationGroup: OperationGroupRecord) => Promise; - afterExecuteOperationGroupAsync?: (operationGroup: OperationGroupRecord) => Promise; + beforeExecuteOperation?: (operation: Operation) => void; + afterExecuteOperation?: (operation: Operation) => void; + beforeExecuteOperationGroup?: (operationGroup: OperationGroupRecord) => void; + afterExecuteOperationGroup?: (operationGroup: OperationGroupRecord) => void; } /** @@ -126,9 +126,7 @@ export class OperationExecutionManager - ): Promise => { + beforeExecute: (operation: Operation): void => { // Initialize group if uninitialized and log the group name const { group, runner } = operation; if (group) { @@ -136,18 +134,18 @@ export class OperationExecutionManager, state: IOperationState - ): Promise => { + ): void => { const { group, runner } = operation; if (group) { group.setOperationAsComplete(operation, state); @@ -165,7 +163,7 @@ export class OperationExecutionManager