From a0f79ea9ad297c9cb1de249dc1c8088537fb5693 Mon Sep 17 00:00:00 2001 From: Chao Guo <10736839+g-chao@users.noreply.github.com> Date: Tue, 18 Jun 2024 17:16:43 -0700 Subject: [PATCH 01/21] feat: rush alerts - print alerts --- .../rush-lib/src/cli/RushCommandLineParser.ts | 8 ++ .../rush-lib/src/utilities/RushAlerts.ts | 75 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 libraries/rush-lib/src/utilities/RushAlerts.ts diff --git a/libraries/rush-lib/src/cli/RushCommandLineParser.ts b/libraries/rush-lib/src/cli/RushCommandLineParser.ts index 733bc14b071..5db7f0ef0c2 100644 --- a/libraries/rush-lib/src/cli/RushCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushCommandLineParser.ts @@ -59,6 +59,7 @@ import { RushSession } from '../pluginFramework/RushSession'; import { PhasedScriptAction } from './scriptActions/PhasedScriptAction'; import type { IBuiltInPluginConfiguration } from '../pluginFramework/PluginLoader/BuiltInPluginLoader'; import { InitSubspaceAction } from './actions/InitSubspaceAction'; +import { RushAlerts } from '../utilities/RushAlerts'; /** * Options for `RushCommandLineParser`. @@ -230,6 +231,13 @@ export class RushCommandLineParser extends CommandLineParser { // This only gets hit if the wrapped execution completes successfully await this.telemetry?.ensureFlushedAsync(); + + // Print out alerts if have after each successful command actions + const rushAlerts: RushAlerts = new RushAlerts({ + rushConfiguration: this.rushConfiguration, + terminal: this._terminal + }); + await rushAlerts.printAlertsAsync(); } private _normalizeOptions(options: Partial): IRushCommandLineParserOptions { diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts new file mode 100644 index 00000000000..3287748c92e --- /dev/null +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { readFile } from 'fs/promises'; +import { existsSync } from 'fs'; +import type { RushConfiguration } from '../api/RushConfiguration'; +import type { Terminal } from '@rushstack/terminal'; + +export interface IRushAlertsOptions { + rushConfiguration: RushConfiguration; + terminal: Terminal; +} + +interface IRushAlerts { + alerts: Array; +} +interface IRushAlert { + title: string; + details: Array; + detailsUrl: string; +} + +export class RushAlerts { + private readonly _rushConfiguration: RushConfiguration; + private readonly _terminal: Terminal; + + public constructor(options: IRushAlertsOptions) { + this._rushConfiguration = options.rushConfiguration; + this._terminal = options.terminal; + } + + private _printMessageInBoxStyle(alert: IRushAlert): void { + const messageToBePrinted: Array = []; + messageToBePrinted.push(alert.title); + messageToBePrinted.push(...alert.details); + messageToBePrinted.push(alert.detailsUrl); + + // Calculate max length for the border + const maxLength: number = messageToBePrinted.reduce((max, line) => Math.max(max, line.length), 0); + + // Add padding for border + const paddedLength: number = maxLength + 4; + + // Create border lines + const borderLine: string = '+'.padEnd(paddedLength - 1, '-') + '+'; + // const emptyLine = '|' + ' '.padEnd(maxLength + 2) + '|'; + + // Print the box + this._terminal.writeLine(borderLine); + messageToBePrinted.forEach((line) => { + const padding: number = maxLength - line.length; + this._terminal.writeLine(`| ${line}${' '.repeat(padding)} |`); + }); + this._terminal.writeLine(borderLine); + } + + public async printAlertsAsync(): Promise { + // let's hardcode this first + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/rush-alerts.json`; + + try { + if (existsSync(rushAlertsStateFilePath)) { + const rushAlertsData: IRushAlerts = JSON.parse((await readFile(rushAlertsStateFilePath)).toString()); + if (rushAlertsData?.alerts.length !== 0) { + for (const alert of rushAlertsData.alerts) { + this._printMessageInBoxStyle(alert); + } + } + } + } catch (e) { + // console the message only in debug mode + this._terminal.writeDebugLine(e.message); + } + } +} From b23b0e5cecab14f603f17a0ede8285652b286949 Mon Sep 17 00:00:00 2001 From: Chao Guo <10736839+g-chao@users.noreply.github.com> Date: Sun, 23 Jun 2024 11:31:24 -0700 Subject: [PATCH 02/21] feat: rush alerts prototype --- common/reviews/api/rush-lib.api.md | 2 + .../common/config/rush/experiments.json | 8 +- .../src/api/ExperimentsConfiguration.ts | 6 + .../rush-lib/src/api/RushConfiguration.ts | 3 +- .../rush-lib/src/cli/RushCommandLineParser.ts | 24 ++- libraries/rush-lib/src/logic/RushConstants.ts | 5 + .../src/schemas/experiments.schema.json | 4 + .../rush-lib/src/utilities/RushAlerts.ts | 141 +++++++++++++++--- 8 files changed, 161 insertions(+), 32 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 1abcde33f6d..ff57da8eff1 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -470,6 +470,7 @@ export interface IExperimentsJson { buildCacheWithAllowWarningsInSuccessfulBuild?: boolean; buildSkipWithAllowWarningsInSuccessfulBuild?: boolean; cleanInstallAfterNpmrcChanges?: boolean; + enablePushRushAlertsToEndUsersInCLI?: boolean; forbidPhantomResolvableNodeModulesFolders?: boolean; generateProjectImpactGraphDuringRushUpdate?: boolean; noChmodFieldInTarHeaderNormalization?: boolean; @@ -1314,6 +1315,7 @@ export class RushConfigurationProject { // @beta export class RushConstants { + static readonly alertsConfigFilename: 'alerts-config.json'; static readonly artifactoryFilename: 'artifactory.json'; static readonly browserApprovedPackagesFilename: 'browser-approved-packages.json'; static readonly buildCacheFilename: 'build-cache.json'; diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json b/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json index ab425270e97..db31fd504fe 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json @@ -87,5 +87,11 @@ * of `_phase:` if they exist. The created child process will be provided with an IPC channel and expected to persist * across invocations. */ - /*[LINE "HYPOTHETICAL"]*/ "useIPCScriptsInWatchMode": true + /*[LINE "HYPOTHETICAL"]*/ "useIPCScriptsInWatchMode": true, + + /** + * (UNDER DEVELOPMENT) The Rush alerts is a mechanism to let Monorepo maintainers be able to push alerts to + * users during their day to day development activities in CLI. + */ + /*[LINE "HYPOTHETICAL"]*/ "enablePushRushAlertsToEndUsersInCLI": true } diff --git a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts index af4b67e29ad..60e498214e7 100644 --- a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts @@ -98,6 +98,12 @@ export interface IExperimentsJson { * across invocations. */ useIPCScriptsInWatchMode?: boolean; + + /** + * (UNDER DEVELOPMENT) The Rush alerts is a mechanism to let Monorepo maintainers be able to push alerts to + * users during their day to day development activities in CLI. + */ + enablePushRushAlertsToEndUsersInCLI?: boolean; } const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); diff --git a/libraries/rush-lib/src/api/RushConfiguration.ts b/libraries/rush-lib/src/api/RushConfiguration.ts index 241f1f8cc57..5bc4f21d085 100644 --- a/libraries/rush-lib/src/api/RushConfiguration.ts +++ b/libraries/rush-lib/src/api/RushConfiguration.ts @@ -71,7 +71,8 @@ const knownRushConfigFilenames: string[] = [ RushConstants.versionPoliciesFilename, RushConstants.rushPluginsConfigFilename, RushConstants.pnpmConfigFilename, - RushConstants.subspacesConfigFilename + RushConstants.subspacesConfigFilename, + RushConstants.alertsConfigFilename ]; /** diff --git a/libraries/rush-lib/src/cli/RushCommandLineParser.ts b/libraries/rush-lib/src/cli/RushCommandLineParser.ts index 5db7f0ef0c2..7cc3876d9c1 100644 --- a/libraries/rush-lib/src/cli/RushCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushCommandLineParser.ts @@ -232,12 +232,24 @@ export class RushCommandLineParser extends CommandLineParser { // This only gets hit if the wrapped execution completes successfully await this.telemetry?.ensureFlushedAsync(); - // Print out alerts if have after each successful command actions - const rushAlerts: RushAlerts = new RushAlerts({ - rushConfiguration: this.rushConfiguration, - terminal: this._terminal - }); - await rushAlerts.printAlertsAsync(); + try { + const { configuration: experiments } = this.rushConfiguration.experimentsConfiguration; + if (experiments.enablePushRushAlertsToEndUsersInCLI) { + // Print out alerts if have after each successful command actions + const rushAlerts: RushAlerts = new RushAlerts({ + rushConfiguration: this.rushConfiguration, + terminal: this._terminal + }); + if (await rushAlerts.isAlertsStateUpToDateAsync()) { + await rushAlerts.printAlertsAsync(); + } else { + await rushAlerts.retrieveAlertsAsync(); + } + } + } catch (error) { + // Only print errors in debug mode + this._terminal.writeDebugLine(error.message); + } } private _normalizeOptions(options: Partial): IRushCommandLineParserOptions { diff --git a/libraries/rush-lib/src/logic/RushConstants.ts b/libraries/rush-lib/src/logic/RushConstants.ts index 201a7523128..64936aae0d2 100644 --- a/libraries/rush-lib/src/logic/RushConstants.ts +++ b/libraries/rush-lib/src/logic/RushConstants.ts @@ -325,4 +325,9 @@ export class RushConstants { * The filename for the last link flag */ public static readonly lastLinkFlagFilename: 'last-link' = 'last-link'; + + /** + * The filename for rush alerts config + */ + public static readonly alertsConfigFilename: 'alerts-config.json' = 'alerts-config.json'; } diff --git a/libraries/rush-lib/src/schemas/experiments.schema.json b/libraries/rush-lib/src/schemas/experiments.schema.json index 38df9737ddc..cd5d9865713 100644 --- a/libraries/rush-lib/src/schemas/experiments.schema.json +++ b/libraries/rush-lib/src/schemas/experiments.schema.json @@ -65,6 +65,10 @@ "useIPCScriptsInWatchMode": { "description": "If true, when running in watch mode, Rush will check for phase scripts named `_phase::ipc` and run them instead of `_phase:` if they exist. The created child process will be provided with an IPC channel and expected to persist across invocations.", "type": "boolean" + }, + "enablePushRushAlertsToEndUsersInCLI": { + "description": "(UNDER DEVELOPMENT) The Rush alerts is a mechanism to let Monorepo maintainers be able to push alerts to users during their day to day development activities in CLI.", + "type": "boolean" } }, "additionalProperties": false diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 3287748c92e..20db3a4f4ac 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -1,20 +1,32 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { readFile } from 'fs/promises'; +import { readFile, writeFile } from 'fs/promises'; import { existsSync } from 'fs'; -import type { RushConfiguration } from '../api/RushConfiguration'; import type { Terminal } from '@rushstack/terminal'; +import type { RushConfiguration } from '../api/RushConfiguration'; export interface IRushAlertsOptions { rushConfiguration: RushConfiguration; terminal: Terminal; } -interface IRushAlerts { - alerts: Array; +interface IRushAlertsConfig { + alerts: Array; } -interface IRushAlert { +interface IRushAlertsConfigEntry { + title: string; + details: Array; + detailsUrl: string; + startTime: string; + endTime: string; + condition?: string; +} +interface IRushAlertsState { + lastUpdateTime: string; + alerts: Array; +} +interface IRushAlertStateEntry { title: string; details: Array; detailsUrl: string; @@ -29,7 +41,101 @@ export class RushAlerts { this._terminal = options.terminal; } - private _printMessageInBoxStyle(alert: IRushAlert): void { + public async isAlertsStateUpToDateAsync(): Promise { + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/alerts-state.json`; + if (!existsSync(rushAlertsStateFilePath)) { + return false; + } + const rushAlertsData: IRushAlertsState = JSON.parse((await readFile(rushAlertsStateFilePath)).toString()); + + if (rushAlertsData.lastUpdateTime) { + const currentTime: Date = new Date(); + const lastUpdateTime: Date = new Date(rushAlertsData.lastUpdateTime); + + const hours: number = (Number(currentTime) - Number(lastUpdateTime)) / (1000 * 60 * 60); + + if (hours > 24) { + return false; + } + } + return true; + } + + public async retrieveAlertsAsync(): Promise { + const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/alerts-config.json`; + try { + if (existsSync(rushAlertsConfigFilePath)) { + const rushAlertsConfig: IRushAlertsConfig = JSON.parse( + (await readFile(rushAlertsConfigFilePath)).toString() + ); + + const validAlerts: Array = []; + if (rushAlertsConfig?.alerts.length !== 0) { + for (const alert of rushAlertsConfig.alerts) { + if (await this._isAlertValidAsync(alert)) { + validAlerts.push({ + title: alert.title, + details: alert.details, + detailsUrl: alert.detailsUrl + }); + } + } + } + + if (validAlerts.length > 0) { + const rushAlertsState: IRushAlertsState = { + lastUpdateTime: new Date().toString(), + alerts: validAlerts + }; + await this._writeRushAlertStateAsync(rushAlertsState); + } + } + } catch (e) { + // console the message only in debug mode + this._terminal.writeDebugLine(e.message); + } + } + + public async printAlertsAsync(): Promise { + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/alerts-state.json`; + + try { + if (existsSync(rushAlertsStateFilePath)) { + const rushAlertsData: IRushAlertsState = JSON.parse( + (await readFile(rushAlertsStateFilePath)).toString() + ); + if (rushAlertsData?.alerts.length !== 0) { + for (const alert of rushAlertsData.alerts) { + this._printMessageInBoxStyle(alert); + } + } + } + } catch (e) { + // console the message only in debug mode + this._terminal.writeDebugLine(e.message); + } + } + + private async _isAlertValidAsync(alert: IRushAlertsConfigEntry): Promise { + const timeNow: Date = new Date(); + if (timeNow < new Date(alert.startTime) || timeNow > new Date(alert.endTime)) { + return false; + } + if (alert.condition) { + const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/${alert.condition}`; + if (!existsSync(conditionScriptPath)) { + this._terminal.writeDebugLine(`${conditionScriptPath} is not exist!`); + return false; + } + + if (!(await require(`${this._rushConfiguration.rushJsonFolder}/${alert.condition}`))()) { + return false; + } + } + return true; + } + + private _printMessageInBoxStyle(alert: IRushAlertStateEntry): void { const messageToBePrinted: Array = []; messageToBePrinted.push(alert.title); messageToBePrinted.push(...alert.details); @@ -43,7 +149,6 @@ export class RushAlerts { // Create border lines const borderLine: string = '+'.padEnd(paddedLength - 1, '-') + '+'; - // const emptyLine = '|' + ' '.padEnd(maxLength + 2) + '|'; // Print the box this._terminal.writeLine(borderLine); @@ -54,22 +159,10 @@ export class RushAlerts { this._terminal.writeLine(borderLine); } - public async printAlertsAsync(): Promise { - // let's hardcode this first - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/rush-alerts.json`; - - try { - if (existsSync(rushAlertsStateFilePath)) { - const rushAlertsData: IRushAlerts = JSON.parse((await readFile(rushAlertsStateFilePath)).toString()); - if (rushAlertsData?.alerts.length !== 0) { - for (const alert of rushAlertsData.alerts) { - this._printMessageInBoxStyle(alert); - } - } - } - } catch (e) { - // console the message only in debug mode - this._terminal.writeDebugLine(e.message); - } + private async _writeRushAlertStateAsync(rushAlertsState: IRushAlertsState): Promise { + await writeFile( + `${this._rushConfiguration.commonTempFolder}/alerts-state.json`, + JSON.stringify(rushAlertsState, null, 2) + ); } } From 190bd8dc13f9a817dc27c379f189cd29d12fa01e Mon Sep 17 00:00:00 2001 From: Chao Guo <10736839+g-chao@users.noreply.github.com> Date: Sun, 23 Jun 2024 11:32:35 -0700 Subject: [PATCH 03/21] chore: rush change --- .../rush/chao-rush-notification_2024-06-23-18-32.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json diff --git a/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json b/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json new file mode 100644 index 00000000000..1c700cfd48c --- /dev/null +++ b/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Prototype for Rush alerts feature", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file From b21836d272f546ee463a046ca13d8033dd2c540a Mon Sep 17 00:00:00 2001 From: Chao Guo <10736839+g-chao@users.noreply.github.com> Date: Sun, 23 Jun 2024 16:20:21 -0700 Subject: [PATCH 04/21] chore: fix bugs --- .../rush-lib/src/utilities/RushAlerts.ts | 52 ++++++++++++------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 20db3a4f4ac..8d0fc7d2178 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.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 { readFile, writeFile } from 'fs/promises'; +import { readFile, writeFile, unlink } from 'fs/promises'; import { existsSync } from 'fs'; import type { Terminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../api/RushConfiguration'; @@ -48,16 +48,19 @@ export class RushAlerts { } const rushAlertsData: IRushAlertsState = JSON.parse((await readFile(rushAlertsStateFilePath)).toString()); - if (rushAlertsData.lastUpdateTime) { - const currentTime: Date = new Date(); - const lastUpdateTime: Date = new Date(rushAlertsData.lastUpdateTime); + if (!rushAlertsData.lastUpdateTime) { + return false; + } - const hours: number = (Number(currentTime) - Number(lastUpdateTime)) / (1000 * 60 * 60); + const currentTime: Date = new Date(); + const lastUpdateTime: Date = new Date(rushAlertsData.lastUpdateTime); - if (hours > 24) { - return false; - } + const hours: number = (Number(currentTime) - Number(lastUpdateTime)) / (1000 * 60 * 60); + + if (hours > 24) { + return false; } + return true; } @@ -82,13 +85,7 @@ export class RushAlerts { } } - if (validAlerts.length > 0) { - const rushAlertsState: IRushAlertsState = { - lastUpdateTime: new Date().toString(), - alerts: validAlerts - }; - await this._writeRushAlertStateAsync(rushAlertsState); - } + await this._writeRushAlertStateAsync(validAlerts); } } catch (e) { // console the message only in debug mode @@ -159,10 +156,25 @@ export class RushAlerts { this._terminal.writeLine(borderLine); } - private async _writeRushAlertStateAsync(rushAlertsState: IRushAlertsState): Promise { - await writeFile( - `${this._rushConfiguration.commonTempFolder}/alerts-state.json`, - JSON.stringify(rushAlertsState, null, 2) - ); + private async _writeRushAlertStateAsync(validAlerts: Array): Promise { + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/alerts-state.json`; + + if (validAlerts.length > 0) { + const rushAlertsState: IRushAlertsState = { + lastUpdateTime: new Date().toString(), + alerts: validAlerts + }; + + await writeFile( + `${this._rushConfiguration.commonTempFolder}/alerts-state.json`, + JSON.stringify(rushAlertsState, null, 2) + ); + } else { + // if no valid alerts + // remove exist alerts state if exist + if (existsSync(rushAlertsStateFilePath)) { + await unlink(rushAlertsStateFilePath); + } + } } } From dc52f2db2b60e5d577e10a7302aefe36f12e5bfd Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 30 Jun 2024 18:52:52 -0700 Subject: [PATCH 05/21] Simplify experiment name --- .../assets/rush-init/common/config/rush/experiments.json | 9 ++++++--- libraries/rush-lib/src/api/ExperimentsConfiguration.ts | 8 +++++--- libraries/rush-lib/src/cli/RushCommandLineParser.ts | 2 +- libraries/rush-lib/src/schemas/experiments.schema.json | 4 ++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json b/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json index db31fd504fe..326b7c42af0 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/experiments.json @@ -82,6 +82,7 @@ * If set to true, Rush will generate a `project-impact-graph.yaml` file in the repository root during `rush update`. */ /*[LINE "HYPOTHETICAL"]*/ "generateProjectImpactGraphDuringRushUpdate": true, + /** * If true, when running in watch mode, Rush will check for phase scripts named `_phase::ipc` and run them instead * of `_phase:` if they exist. The created child process will be provided with an IPC channel and expected to persist @@ -90,8 +91,10 @@ /*[LINE "HYPOTHETICAL"]*/ "useIPCScriptsInWatchMode": true, /** - * (UNDER DEVELOPMENT) The Rush alerts is a mechanism to let Monorepo maintainers be able to push alerts to - * users during their day to day development activities in CLI. + * (UNDER DEVELOPMENT) The Rush alerts feature provides a way to send announcements to engineers + * working in the monorepo, by printing directly in the user's shell window when they invoke Rush commands. + * This ensures that important notices will be seen by anyone doing active development, since people often + * ignore normal discussion group messages or don't know to subscribe. */ - /*[LINE "HYPOTHETICAL"]*/ "enablePushRushAlertsToEndUsersInCLI": true + /*[LINE "HYPOTHETICAL"]*/ "rushAlerts": true } diff --git a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts index 60e498214e7..0e66fa3dc1c 100644 --- a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts @@ -100,10 +100,12 @@ export interface IExperimentsJson { useIPCScriptsInWatchMode?: boolean; /** - * (UNDER DEVELOPMENT) The Rush alerts is a mechanism to let Monorepo maintainers be able to push alerts to - * users during their day to day development activities in CLI. + * (UNDER DEVELOPMENT) The Rush alerts feature provides a way to send announcements to engineers + * working in the monorepo, by printing directly in the user's shell window when they invoke Rush commands. + * This ensures that important notices will be seen by anyone doing active development, since people often + * ignore normal discussion group messages or don't know to subscribe. */ - enablePushRushAlertsToEndUsersInCLI?: boolean; + rushAlerts?: boolean; } const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); diff --git a/libraries/rush-lib/src/cli/RushCommandLineParser.ts b/libraries/rush-lib/src/cli/RushCommandLineParser.ts index 7cc3876d9c1..84419da066e 100644 --- a/libraries/rush-lib/src/cli/RushCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushCommandLineParser.ts @@ -234,7 +234,7 @@ export class RushCommandLineParser extends CommandLineParser { try { const { configuration: experiments } = this.rushConfiguration.experimentsConfiguration; - if (experiments.enablePushRushAlertsToEndUsersInCLI) { + if (experiments.rushAlerts) { // Print out alerts if have after each successful command actions const rushAlerts: RushAlerts = new RushAlerts({ rushConfiguration: this.rushConfiguration, diff --git a/libraries/rush-lib/src/schemas/experiments.schema.json b/libraries/rush-lib/src/schemas/experiments.schema.json index cd5d9865713..c2026513fdb 100644 --- a/libraries/rush-lib/src/schemas/experiments.schema.json +++ b/libraries/rush-lib/src/schemas/experiments.schema.json @@ -66,8 +66,8 @@ "description": "If true, when running in watch mode, Rush will check for phase scripts named `_phase::ipc` and run them instead of `_phase:` if they exist. The created child process will be provided with an IPC channel and expected to persist across invocations.", "type": "boolean" }, - "enablePushRushAlertsToEndUsersInCLI": { - "description": "(UNDER DEVELOPMENT) The Rush alerts is a mechanism to let Monorepo maintainers be able to push alerts to users during their day to day development activities in CLI.", + "rushAlerts": { + "description": "(UNDER DEVELOPMENT) The Rush alerts feature provides a way to send announcements to engineers working in the monorepo, by printing directly in the user's shell window when they invoke Rush commands. This ensures that important notices will be seen by anyone doing active development, since people often ignore normal discussion group messages or don't know to subscribe.", "type": "boolean" } }, From 3a43c589b90b34e200b2af42e75b544230145d39 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 30 Jun 2024 18:57:33 -0700 Subject: [PATCH 06/21] Rename alerts-config.json to rush-alerts.json --- libraries/rush-lib/src/api/RushConfiguration.ts | 2 +- libraries/rush-lib/src/logic/RushConstants.ts | 2 +- libraries/rush-lib/src/utilities/RushAlerts.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/rush-lib/src/api/RushConfiguration.ts b/libraries/rush-lib/src/api/RushConfiguration.ts index 5bc4f21d085..ab71426f823 100644 --- a/libraries/rush-lib/src/api/RushConfiguration.ts +++ b/libraries/rush-lib/src/api/RushConfiguration.ts @@ -72,7 +72,7 @@ const knownRushConfigFilenames: string[] = [ RushConstants.rushPluginsConfigFilename, RushConstants.pnpmConfigFilename, RushConstants.subspacesConfigFilename, - RushConstants.alertsConfigFilename + RushConstants.rushAlertsConfigFilename ]; /** diff --git a/libraries/rush-lib/src/logic/RushConstants.ts b/libraries/rush-lib/src/logic/RushConstants.ts index 64936aae0d2..bd27bae987e 100644 --- a/libraries/rush-lib/src/logic/RushConstants.ts +++ b/libraries/rush-lib/src/logic/RushConstants.ts @@ -329,5 +329,5 @@ export class RushConstants { /** * The filename for rush alerts config */ - public static readonly alertsConfigFilename: 'alerts-config.json' = 'alerts-config.json'; + public static readonly rushAlertsConfigFilename: 'rush-alerts.json' = 'rush-alerts.json'; } diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 8d0fc7d2178..fba3cd546f3 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -65,7 +65,7 @@ export class RushAlerts { } public async retrieveAlertsAsync(): Promise { - const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/alerts-config.json`; + const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/rush-alerts.json`; try { if (existsSync(rushAlertsConfigFilePath)) { const rushAlertsConfig: IRushAlertsConfig = JSON.parse( From 9cce6466a1bd86163bb3ff57c78dfdaa272a9dde Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 30 Jun 2024 23:20:42 -0700 Subject: [PATCH 07/21] Draft of `rush init` template --- .../common/config/rush/rush-alerts.json | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json b/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json new file mode 100644 index 00000000000..293f506606c --- /dev/null +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json @@ -0,0 +1,90 @@ +/** + * This configuration file manages the Rush alerts feature. + * More documentation is available on the Rush website: https://rushjs.io + */ +{ + "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/rush-alerts.schema.json", + + /** + * Settings such as `startTime` and `endTime` will use this timezone. + * If omitted, the default timezone is UTC (`+00:00`). + */ + "timezone": "-08:00", + + /** + * An array of alert messages and conditions for triggering them. + */ + "alerts": [ + /*[BEGIN "DEMO"]*/ + { + /** + * When the alert is displayed, this title will appear at the top of the message box. + * It should be a single line of text, as concise as possible. + */ + "title": "Node.js upgrade soon!", + + /** + * When the alert is displayed, this text appears in the message box. To make the + * JSON file more readable, if the text is longer than one line, you can instead provide + * an array of strings that will be concatenated. Your text may contain newline characters, + * but generally this is unnecessary because word-wrapping is automatically applied. + */ + "message": [ + "This Thursday, we will complete the Node.js version upgrade. Any pipelines that", + " still have not upgraded will be temporarily disabled." + ], + + /** + * (OPTIONAL) To avoid spamming users, the `title` and `message` settings should be kept + * as concise as possible. If you need to provide more detail, use this setting to + * print a hyperlink to a web page with further guidance. + */ + /*[LINE "HYPOTHETICAL"]*/ "detailsUrl": "https://contoso.com/team-wiki/2024-01-01-migration", + + /** + * (OPTIONAL) If `startTime` is specified, then this alert will not be shown prior to + * that time. + * + * Keep in mind that the alert is not guaranteed to be shown at this time, or at all: + * Alerts are only displayed after a Rush command has triggered fetching of the + * latest rush-alerts.json configuration. Also, display of alerts is throttled to + * avoid spamming the user with too many messages. If you need to test your alert, + * set the environment variable `RUSH_ALERTS_DEBUG=1` to disable throttling. + * + * The `startTime` should be specified as `YYYY-MM-DD HH:MM` using 24 hour time format, + * or else `YYYY-MM-DD` in which case the time part will be `00:00` (start of that day). + * The time zone is obtained from the `timezone` setting above. + */ + /*[LINE "HYPOTHETICAL"]*/ "startTime": "2024-01-01 15:00", + + /** + * (OPTIONAL) This alert will not be shown if the current time is later than `endTime`. + * The format is the same as `startTime`. + */ + /*[LINE "HYPOTHETICAL"]*/ "endTime": "2024-01-05", + + /** + * (OPTIONAL) The filename of a script that determines whether this alert can be shown, + * found in the "common/config/rush/alert-scripts" folder. The script must define + * a CommonJS export named `canShowAlert` that returns a boolean value, for example: + * + * ``` + * module.exports.canShowAlert = function () { + * // (your logic goes here) + * return true; + * } + * ``` + * + * Rush will invoke this script with the working directory set to the monorepo root folder, + * with no guarantee that `rush install` has been run. To ensure up-to-date alerts, Rush + * may fetch and checkout the "common/config/rush-alerts" folder in an unpredictable temporary + * path. Therefore, your script should avoid importing dependencies from outside its folder, + * generally be kept as simple and reliable and quick as possible. For more complex conditions, + * we suggest to design some other process that prepares a data file or environment variable + * that can be cheaply checked by your condition script. + */ + /*[LINE "HYPOTHETICAL"]*/ "conditionScript": "rush-alert-node-upgrade.js" + } + /*[END "DEMO"]*/ + ] +} From 39fdcbbe5276dccc8e9698434a379f8bffeec9ce Mon Sep 17 00:00:00 2001 From: Chao Guo <10736839+g-chao@users.noreply.github.com> Date: Mon, 1 Jul 2024 14:38:35 -0700 Subject: [PATCH 08/21] feat: register rushAlerts template in rush init action --- common/reviews/api/rush-lib.api.md | 4 ++-- libraries/rush-lib/src/cli/actions/InitAction.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index ff57da8eff1..6ce2331ea6c 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -470,12 +470,12 @@ export interface IExperimentsJson { buildCacheWithAllowWarningsInSuccessfulBuild?: boolean; buildSkipWithAllowWarningsInSuccessfulBuild?: boolean; cleanInstallAfterNpmrcChanges?: boolean; - enablePushRushAlertsToEndUsersInCLI?: boolean; forbidPhantomResolvableNodeModulesFolders?: boolean; generateProjectImpactGraphDuringRushUpdate?: boolean; noChmodFieldInTarHeaderNormalization?: boolean; omitImportersFromPreventManualShrinkwrapChanges?: boolean; printEventHooksOutputToConsole?: boolean; + rushAlerts?: boolean; useIPCScriptsInWatchMode?: boolean; usePnpmFrozenLockfileForRushInstall?: boolean; usePnpmLockfileOnlyThenFrozenLockfileForRushUpdate?: boolean; @@ -1315,7 +1315,6 @@ export class RushConfigurationProject { // @beta export class RushConstants { - static readonly alertsConfigFilename: 'alerts-config.json'; static readonly artifactoryFilename: 'artifactory.json'; static readonly browserApprovedPackagesFilename: 'browser-approved-packages.json'; static readonly buildCacheFilename: 'build-cache.json'; @@ -1357,6 +1356,7 @@ export class RushConstants { static readonly projectShrinkwrapFilename: 'shrinkwrap-deps.json'; static readonly rebuildCommandName: 'rebuild'; static readonly repoStateFilename: 'repo-state.json'; + static readonly rushAlertsConfigFilename: 'rush-alerts.json'; static readonly rushJsonFilename: 'rush.json'; static readonly rushLogsFolderName: 'rush-logs'; static readonly rushPackageName: '@microsoft/rush'; diff --git a/libraries/rush-lib/src/cli/actions/InitAction.ts b/libraries/rush-lib/src/cli/actions/InitAction.ts index ac5273ff94f..bad633ceb58 100644 --- a/libraries/rush-lib/src/cli/actions/InitAction.ts +++ b/libraries/rush-lib/src/cli/actions/InitAction.ts @@ -143,7 +143,8 @@ export class InitAction extends BaseConfiglessRushAction { ]; const experimentalTemplateFilePaths: string[] = [ - `common/config/rush/${RushConstants.subspacesConfigFilename}` + `common/config/rush/${RushConstants.subspacesConfigFilename}`, + 'common/config/rush/rush-alerts.json' ]; if (this._experimentsParameter.value) { From f8e970f7effe69a2c9db62555e35fa8c95ad2622 Mon Sep 17 00:00:00 2001 From: Chao Guo <10736839+g-chao@users.noreply.github.com> Date: Wed, 3 Jul 2024 13:26:14 -0700 Subject: [PATCH 09/21] feat: add rush alerts schema --- .../src/schemas/rush-alerts.schema.json | 59 +++++++++++++++++++ .../rush-lib/src/utilities/RushAlerts.ts | 37 +++++++----- 2 files changed, 80 insertions(+), 16 deletions(-) create mode 100644 libraries/rush-lib/src/schemas/rush-alerts.schema.json diff --git a/libraries/rush-lib/src/schemas/rush-alerts.schema.json b/libraries/rush-lib/src/schemas/rush-alerts.schema.json new file mode 100644 index 00000000000..4e08bafcc19 --- /dev/null +++ b/libraries/rush-lib/src/schemas/rush-alerts.schema.json @@ -0,0 +1,59 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Rush rush-alerts.json file", + "description": "This configuration file provides settings to rush alerts feature.", + "type": "object", + "additionalProperties": false, + "properties": { + "timezone": { + "description": "Settings such as `startTime` and `endTime` will use this timezone. \n\nIf omitted, the default timezone is UTC (`+00:00`).", + "type": "string" + }, + "alerts": { + "description": "An array of alert messages and conditions for triggering them.", + "items": { + "$ref": "#/definitions/IAlert" + }, + "type": "array" + } + }, + "definitions": { + "IAlert": { + "type": "object", + "properties": { + "title": { + "description": "When the alert is displayed, this title will appear at the top of the message box. It should be a single line of text, as concise as possible.", + "type": "string" + }, + "message": { + "description": "When the alert is displayed, this text appears in the message box. \n\nTo make the JSON file more readable, if the text is longer than one line, you can instead provide an array of strings that will be concatenated. \n\nYour text may contain newline characters, but generally this is unnecessary because word-wrapping is automatically applied.", + "items": { + "$ref": "#/definitions/IAlertMessage" + }, + "type": "array" + }, + "detailsUrl": { + "description": "(OPTIONAL) To avoid spamming users, the `title` and `message` settings should be kept as concise as possible. \n\nIf you need to provide more detail, use this setting to print a hyperlink to a web page with further guidance.", + "type": "string" + }, + "startTime": { + "description": "(OPTIONAL) If `startTime` is specified, then this alert will not be shown prior to that time. \n\nKeep in mind that the alert is not guaranteed to be shown at this time, or at all. Alerts are only displayed after a Rush command has triggered fetching of the latest rush-alerts.json configuration. \n\nAlso, display of alerts is throttled to avoid spamming the user with too many messages. \n\nIf you need to test your alert, set the environment variable `RUSH_ALERTS_DEBUG=1` to disable throttling. \n\nThe `startTime` should be specified as `YYYY-MM-DD HH:MM` using 24 hour time format, or else `YYYY-MM-DD` in which case the time part will be `00:00` (start of that day). The time zone is obtained from the `timezone` setting above.", + "type": "string" + }, + "endTime": { + "description": "(OPTIONAL) This alert will not be shown if the current time is later than `endTime`. \n\nThe format is the same as `startTime`.", + "type": "string" + }, + "conditionScript": { + "description": "(OPTIONAL) The filename of a script that determines whether this alert can be shown, found in the 'common/config/rush/alert-scripts' folder. \n\nThe script must define a CommonJS export named `canShowAlert` that returns a boolean value, for example: \n\n`module.exports.canShowAlert = function () { // (your logic goes here) return true; }`. \n\nRush will invoke this script with the working directory set to the monorepo root folder, with no guarantee that `rush install` has been run. \n\nTo ensure up-to-date alerts, Rush may fetch and checkout the 'common/config/rush-alerts' folder in an unpredictable temporary path. Therefore, your script should avoid importing dependencies from outside its folder, generally be kept as simple and reliable and quick as possible. \n\nFor more complex conditions, we suggest to design some other process that prepares a data file or environment variable that can be cheaply checked by your condition script.", + "type": "string" + } + }, + "required": ["title", "message"] + }, + "IAlertMessage": { + "description": "Message body", + "type": "string" + } + } +} diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index fba3cd546f3..2c029517acd 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -5,6 +5,8 @@ import { readFile, writeFile, unlink } from 'fs/promises'; import { existsSync } from 'fs'; import type { Terminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../api/RushConfiguration'; +import { JsonFile, JsonSchema } from '@rushstack/node-core-library'; +import rushAlertsSchemaJson from '../schemas/rush-alerts.schema.json'; export interface IRushAlertsOptions { rushConfiguration: RushConfiguration; @@ -16,11 +18,11 @@ interface IRushAlertsConfig { } interface IRushAlertsConfigEntry { title: string; - details: Array; + message: Array; detailsUrl: string; startTime: string; endTime: string; - condition?: string; + conditionScript?: string; } interface IRushAlertsState { lastUpdateTime: string; @@ -28,13 +30,16 @@ interface IRushAlertsState { } interface IRushAlertStateEntry { title: string; - details: Array; + message: Array; detailsUrl: string; } export class RushAlerts { private readonly _rushConfiguration: RushConfiguration; private readonly _terminal: Terminal; + private static _rushAlertsJsonSchema: JsonSchema = JsonSchema.fromLoadedObject(rushAlertsSchemaJson); + private static __rushAlertsConfigFileName: string = 'rush-alerts.json'; + private static __rushAlertsStateFileName: string = 'rush-alerts-state.json'; public constructor(options: IRushAlertsOptions) { this._rushConfiguration = options.rushConfiguration; @@ -42,7 +47,7 @@ export class RushAlerts { } public async isAlertsStateUpToDateAsync(): Promise { - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/alerts-state.json`; + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; if (!existsSync(rushAlertsStateFilePath)) { return false; } @@ -65,20 +70,20 @@ export class RushAlerts { } public async retrieveAlertsAsync(): Promise { - const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/rush-alerts.json`; + const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/${RushAlerts.__rushAlertsConfigFileName}`; try { if (existsSync(rushAlertsConfigFilePath)) { - const rushAlertsConfig: IRushAlertsConfig = JSON.parse( - (await readFile(rushAlertsConfigFilePath)).toString() + const rushAlertsConfig: IRushAlertsConfig = JsonFile.loadAndValidate( + rushAlertsConfigFilePath, + RushAlerts._rushAlertsJsonSchema ); - const validAlerts: Array = []; if (rushAlertsConfig?.alerts.length !== 0) { for (const alert of rushAlertsConfig.alerts) { if (await this._isAlertValidAsync(alert)) { validAlerts.push({ title: alert.title, - details: alert.details, + message: alert.message, detailsUrl: alert.detailsUrl }); } @@ -94,7 +99,7 @@ export class RushAlerts { } public async printAlertsAsync(): Promise { - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/alerts-state.json`; + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; try { if (existsSync(rushAlertsStateFilePath)) { @@ -118,14 +123,14 @@ export class RushAlerts { if (timeNow < new Date(alert.startTime) || timeNow > new Date(alert.endTime)) { return false; } - if (alert.condition) { - const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/${alert.condition}`; + if (alert.conditionScript) { + const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/${alert.conditionScript}`; if (!existsSync(conditionScriptPath)) { this._terminal.writeDebugLine(`${conditionScriptPath} is not exist!`); return false; } - if (!(await require(`${this._rushConfiguration.rushJsonFolder}/${alert.condition}`))()) { + if (!(await require(`${this._rushConfiguration.rushJsonFolder}/${alert.conditionScript}`))()) { return false; } } @@ -135,7 +140,7 @@ export class RushAlerts { private _printMessageInBoxStyle(alert: IRushAlertStateEntry): void { const messageToBePrinted: Array = []; messageToBePrinted.push(alert.title); - messageToBePrinted.push(...alert.details); + messageToBePrinted.push(...alert.message); messageToBePrinted.push(alert.detailsUrl); // Calculate max length for the border @@ -157,7 +162,7 @@ export class RushAlerts { } private async _writeRushAlertStateAsync(validAlerts: Array): Promise { - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/alerts-state.json`; + const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; if (validAlerts.length > 0) { const rushAlertsState: IRushAlertsState = { @@ -166,7 +171,7 @@ export class RushAlerts { }; await writeFile( - `${this._rushConfiguration.commonTempFolder}/alerts-state.json`, + `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`, JSON.stringify(rushAlertsState, null, 2) ); } else { From 427644ca7a5714a6d2307424255842215f9f347b Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 18:13:16 -0700 Subject: [PATCH 10/21] Fix an issue where rush-alerts.json could not be loaded when the "$schema" field is present; fix an issue where loading errors were silently discarded --- .../rush-lib/src/cli/RushCommandLineParser.ts | 46 ++++++++------ .../src/schemas/rush-alerts.schema.json | 5 ++ .../rush-lib/src/utilities/RushAlerts.ts | 63 +++++++++---------- 3 files changed, 60 insertions(+), 54 deletions(-) diff --git a/libraries/rush-lib/src/cli/RushCommandLineParser.ts b/libraries/rush-lib/src/cli/RushCommandLineParser.ts index 84419da066e..74893b48952 100644 --- a/libraries/rush-lib/src/cli/RushCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushCommandLineParser.ts @@ -223,6 +223,33 @@ export class RushCommandLineParser extends CommandLineParser { try { await this._wrapOnExecuteAsync(); + + try { + const { configuration: experiments } = this.rushConfiguration.experimentsConfiguration; + if (experiments.rushAlerts) { + this._terminal.writeDebugLine('Checking Rush alerts...'); + // Print out alerts if have after each successful command actions + const rushAlerts: RushAlerts = new RushAlerts({ + rushConfiguration: this.rushConfiguration, + terminal: this._terminal + }); + if (await rushAlerts.isAlertsStateUpToDateAsync()) { + await rushAlerts.printAlertsAsync(); + } else { + await rushAlerts.retrieveAlertsAsync(); + } + } + } catch (error) { + if (error instanceof AlreadyReportedError) { + throw error; + } + // Generally the RushAlerts implementation should handle its own error reporting; if not, + // clarify the source, since the Rush Alerts behavior is nondeterministic and may not repro easily: + this._terminal.writeErrorLine(`\nAn unexpected error was encountered by the Rush alerts feature:`); + this._terminal.writeErrorLine(error.message); + throw new AlreadyReportedError(); + } + // If we make it here, everything went fine, so reset the exit code back to 0 process.exitCode = 0; } catch (error) { @@ -231,25 +258,6 @@ export class RushCommandLineParser extends CommandLineParser { // This only gets hit if the wrapped execution completes successfully await this.telemetry?.ensureFlushedAsync(); - - try { - const { configuration: experiments } = this.rushConfiguration.experimentsConfiguration; - if (experiments.rushAlerts) { - // Print out alerts if have after each successful command actions - const rushAlerts: RushAlerts = new RushAlerts({ - rushConfiguration: this.rushConfiguration, - terminal: this._terminal - }); - if (await rushAlerts.isAlertsStateUpToDateAsync()) { - await rushAlerts.printAlertsAsync(); - } else { - await rushAlerts.retrieveAlertsAsync(); - } - } - } catch (error) { - // Only print errors in debug mode - this._terminal.writeDebugLine(error.message); - } } private _normalizeOptions(options: Partial): IRushCommandLineParserOptions { diff --git a/libraries/rush-lib/src/schemas/rush-alerts.schema.json b/libraries/rush-lib/src/schemas/rush-alerts.schema.json index 4e08bafcc19..e36ac35832d 100644 --- a/libraries/rush-lib/src/schemas/rush-alerts.schema.json +++ b/libraries/rush-lib/src/schemas/rush-alerts.schema.json @@ -5,6 +5,11 @@ "type": "object", "additionalProperties": false, "properties": { + "$schema": { + "description": "Part of the JSON Schema standard, this optional keyword declares the URL of the schema that the file conforms to. Editors may download the schema and use it to perform syntax highlighting.", + "type": "string" + }, + "timezone": { "description": "Settings such as `startTime` and `endTime` will use this timezone. \n\nIf omitted, the default timezone is UTC (`+00:00`).", "type": "string" diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 2c029517acd..2c8bab7c8d6 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -5,7 +5,7 @@ import { readFile, writeFile, unlink } from 'fs/promises'; import { existsSync } from 'fs'; import type { Terminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../api/RushConfiguration'; -import { JsonFile, JsonSchema } from '@rushstack/node-core-library'; +import { JsonFile, JsonSchema, JsonSyntax } from '@rushstack/node-core-library'; import rushAlertsSchemaJson from '../schemas/rush-alerts.schema.json'; export interface IRushAlertsOptions { @@ -51,7 +51,9 @@ export class RushAlerts { if (!existsSync(rushAlertsStateFilePath)) { return false; } - const rushAlertsData: IRushAlertsState = JSON.parse((await readFile(rushAlertsStateFilePath)).toString()); + const rushAlertsData: IRushAlertsState = await JsonFile.loadAsync(rushAlertsStateFilePath, { + jsonSyntax: JsonSyntax.Strict + }); if (!rushAlertsData.lastUpdateTime) { return false; @@ -71,50 +73,41 @@ export class RushAlerts { public async retrieveAlertsAsync(): Promise { const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/${RushAlerts.__rushAlertsConfigFileName}`; - try { - if (existsSync(rushAlertsConfigFilePath)) { - const rushAlertsConfig: IRushAlertsConfig = JsonFile.loadAndValidate( - rushAlertsConfigFilePath, - RushAlerts._rushAlertsJsonSchema - ); - const validAlerts: Array = []; - if (rushAlertsConfig?.alerts.length !== 0) { - for (const alert of rushAlertsConfig.alerts) { - if (await this._isAlertValidAsync(alert)) { - validAlerts.push({ - title: alert.title, - message: alert.message, - detailsUrl: alert.detailsUrl - }); - } + + if (existsSync(rushAlertsConfigFilePath)) { + const rushAlertsConfig: IRushAlertsConfig = JsonFile.loadAndValidate( + rushAlertsConfigFilePath, + RushAlerts._rushAlertsJsonSchema + ); + const validAlerts: Array = []; + if (rushAlertsConfig?.alerts.length !== 0) { + for (const alert of rushAlertsConfig.alerts) { + if (await this._isAlertValidAsync(alert)) { + validAlerts.push({ + title: alert.title, + message: alert.message, + detailsUrl: alert.detailsUrl + }); } } - - await this._writeRushAlertStateAsync(validAlerts); } - } catch (e) { - // console the message only in debug mode - this._terminal.writeDebugLine(e.message); + + await this._writeRushAlertStateAsync(validAlerts); } } public async printAlertsAsync(): Promise { const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; - try { - if (existsSync(rushAlertsStateFilePath)) { - const rushAlertsData: IRushAlertsState = JSON.parse( - (await readFile(rushAlertsStateFilePath)).toString() - ); - if (rushAlertsData?.alerts.length !== 0) { - for (const alert of rushAlertsData.alerts) { - this._printMessageInBoxStyle(alert); - } + if (existsSync(rushAlertsStateFilePath)) { + const rushAlertsData: IRushAlertsState = JSON.parse( + (await readFile(rushAlertsStateFilePath)).toString() + ); + if (rushAlertsData?.alerts.length !== 0) { + for (const alert of rushAlertsData.alerts) { + this._printMessageInBoxStyle(alert); } } - } catch (e) { - // console the message only in debug mode - this._terminal.writeDebugLine(e.message); } } From f02cd426921ccf7e1538dec93a95454333b7f19c Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 18:17:33 -0700 Subject: [PATCH 11/21] Use node-core-library APIs which provide better error handling --- .../rush-lib/src/utilities/RushAlerts.ts | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 2c8bab7c8d6..0647897e491 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -1,11 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { readFile, writeFile, unlink } from 'fs/promises'; -import { existsSync } from 'fs'; import type { Terminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../api/RushConfiguration'; -import { JsonFile, JsonSchema, JsonSyntax } from '@rushstack/node-core-library'; +import { FileSystem, JsonFile, JsonSchema, JsonSyntax } from '@rushstack/node-core-library'; import rushAlertsSchemaJson from '../schemas/rush-alerts.schema.json'; export interface IRushAlertsOptions { @@ -48,7 +46,7 @@ export class RushAlerts { public async isAlertsStateUpToDateAsync(): Promise { const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; - if (!existsSync(rushAlertsStateFilePath)) { + if (!(await FileSystem.existsAsync(rushAlertsStateFilePath))) { return false; } const rushAlertsData: IRushAlertsState = await JsonFile.loadAsync(rushAlertsStateFilePath, { @@ -74,7 +72,7 @@ export class RushAlerts { public async retrieveAlertsAsync(): Promise { const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/${RushAlerts.__rushAlertsConfigFileName}`; - if (existsSync(rushAlertsConfigFilePath)) { + if (await FileSystem.existsAsync(rushAlertsConfigFilePath)) { const rushAlertsConfig: IRushAlertsConfig = JsonFile.loadAndValidate( rushAlertsConfigFilePath, RushAlerts._rushAlertsJsonSchema @@ -99,10 +97,10 @@ export class RushAlerts { public async printAlertsAsync(): Promise { const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; - if (existsSync(rushAlertsStateFilePath)) { - const rushAlertsData: IRushAlertsState = JSON.parse( - (await readFile(rushAlertsStateFilePath)).toString() - ); + if (await FileSystem.existsAsync(rushAlertsStateFilePath)) { + const rushAlertsData: IRushAlertsState = await JsonFile.loadAsync(rushAlertsStateFilePath, { + jsonSyntax: JsonSyntax.Strict + }); if (rushAlertsData?.alerts.length !== 0) { for (const alert of rushAlertsData.alerts) { this._printMessageInBoxStyle(alert); @@ -118,7 +116,7 @@ export class RushAlerts { } if (alert.conditionScript) { const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/${alert.conditionScript}`; - if (!existsSync(conditionScriptPath)) { + if (!(await FileSystem.existsAsync(conditionScriptPath))) { this._terminal.writeDebugLine(`${conditionScriptPath} is not exist!`); return false; } @@ -163,16 +161,14 @@ export class RushAlerts { alerts: validAlerts }; - await writeFile( - `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`, - JSON.stringify(rushAlertsState, null, 2) + await JsonFile.saveAsync( + rushAlertsState, + `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}` ); } else { // if no valid alerts // remove exist alerts state if exist - if (existsSync(rushAlertsStateFilePath)) { - await unlink(rushAlertsStateFilePath); - } + await FileSystem.deleteFileAsync(rushAlertsStateFilePath); } } } From 25cd43fda18d9a08418c62564a0c77c93ecab390 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 18:29:10 -0700 Subject: [PATCH 12/21] Fix a bug where RushAlerts did not work correctly when optional settings were omitted in rush-alerts.json --- .../rush-lib/src/utilities/RushAlerts.ts | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 0647897e491..786ea937f87 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -50,7 +50,7 @@ export class RushAlerts { return false; } const rushAlertsData: IRushAlertsState = await JsonFile.loadAsync(rushAlertsStateFilePath, { - jsonSyntax: JsonSyntax.Strict + jsonSyntax: JsonSyntax.JsonWithComments }); if (!rushAlertsData.lastUpdateTime) { @@ -109,15 +109,35 @@ export class RushAlerts { } } + private static _parseDate(dateString: string): Date { + const parsedDate: Date = new Date(dateString); + if (isNaN(parsedDate.getTime())) { + throw new Error(`Invalid date/time value ${JSON.stringify(dateString)}`); + } + return parsedDate; + } + private async _isAlertValidAsync(alert: IRushAlertsConfigEntry): Promise { const timeNow: Date = new Date(); - if (timeNow < new Date(alert.startTime) || timeNow > new Date(alert.endTime)) { - return false; + + if (alert.startTime) { + const startTime: Date = RushAlerts._parseDate(alert.startTime); + if (timeNow < startTime) { + return false; + } } + + if (alert.endTime) { + const endTime: Date = RushAlerts._parseDate(alert.endTime); + if (timeNow > endTime) { + return false; + } + } + if (alert.conditionScript) { const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/${alert.conditionScript}`; if (!(await FileSystem.existsAsync(conditionScriptPath))) { - this._terminal.writeDebugLine(`${conditionScriptPath} is not exist!`); + this._terminal.writeDebugLine(`${conditionScriptPath} does not exist`); return false; } @@ -163,7 +183,12 @@ export class RushAlerts { await JsonFile.saveAsync( rushAlertsState, - `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}` + `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`, + { + ignoreUndefinedValues: true, + headerComment: 'THIS FILE IS MACHINE-GENERATED -- DO NOT MODIFY', + jsonSyntax: JsonSyntax.JsonWithComments + } ); } else { // if no valid alerts From cb69ea96b7f42331413290936d27c5e1e3b632c4 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 20:03:59 -0700 Subject: [PATCH 13/21] Invoke the conditionScript with robust error-checking and report its execution time --- .../common/config/rush/rush-alerts.json | 2 +- libraries/rush-lib/src/logic/RushConstants.ts | 7 +- .../rush-lib/src/utilities/RushAlerts.ts | 122 ++++++++++++------ 3 files changed, 92 insertions(+), 39 deletions(-) diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json b/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json index 293f506606c..f880525e11a 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/rush-alerts.json @@ -83,7 +83,7 @@ * we suggest to design some other process that prepares a data file or environment variable * that can be cheaply checked by your condition script. */ - /*[LINE "HYPOTHETICAL"]*/ "conditionScript": "rush-alert-node-upgrade.js" + /*[LINE "HYPOTHETICAL"]*/ "conditionScript": "rush-alexrt-node-upgrade.js" } /*[END "DEMO"]*/ ] diff --git a/libraries/rush-lib/src/logic/RushConstants.ts b/libraries/rush-lib/src/logic/RushConstants.ts index bd27bae987e..ad0099d7cc1 100644 --- a/libraries/rush-lib/src/logic/RushConstants.ts +++ b/libraries/rush-lib/src/logic/RushConstants.ts @@ -327,7 +327,12 @@ export class RushConstants { public static readonly lastLinkFlagFilename: 'last-link' = 'last-link'; /** - * The filename for rush alerts config + * The filename for the Rush alerts config file. */ public static readonly rushAlertsConfigFilename: 'rush-alerts.json' = 'rush-alerts.json'; + + /** + * The filename for the machine-generated file that tracks state for Rush alerts. + */ + public static readonly rushAlertsStateFilename: 'rush-alerts-state.json' = 'rush-alerts-state.json'; } diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 786ea937f87..02863d762f9 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -5,6 +5,7 @@ import type { Terminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../api/RushConfiguration'; import { FileSystem, JsonFile, JsonSchema, JsonSyntax } from '@rushstack/node-core-library'; import rushAlertsSchemaJson from '../schemas/rush-alerts.schema.json'; +import { RushConstants } from '../logic/RushConstants'; export interface IRushAlertsOptions { rushConfiguration: RushConfiguration; @@ -36,29 +37,35 @@ export class RushAlerts { private readonly _rushConfiguration: RushConfiguration; private readonly _terminal: Terminal; private static _rushAlertsJsonSchema: JsonSchema = JsonSchema.fromLoadedObject(rushAlertsSchemaJson); - private static __rushAlertsConfigFileName: string = 'rush-alerts.json'; - private static __rushAlertsStateFileName: string = 'rush-alerts-state.json'; + + public readonly rushAlertsStateFilePath: string; public constructor(options: IRushAlertsOptions) { this._rushConfiguration = options.rushConfiguration; this._terminal = options.terminal; + + this.rushAlertsStateFilePath = `${this._rushConfiguration.commonTempFolder}/${RushConstants.rushAlertsConfigFilename}`; } - public async isAlertsStateUpToDateAsync(): Promise { - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; - if (!(await FileSystem.existsAsync(rushAlertsStateFilePath))) { - return false; + private async _loadRushAlertsStateAsync(): Promise { + if (!(await FileSystem.existsAsync(this.rushAlertsStateFilePath))) { + return undefined; } - const rushAlertsData: IRushAlertsState = await JsonFile.loadAsync(rushAlertsStateFilePath, { + const rushAlertsState: IRushAlertsState = await JsonFile.loadAsync(this.rushAlertsStateFilePath, { jsonSyntax: JsonSyntax.JsonWithComments }); + return rushAlertsState; + } + + public async isAlertsStateUpToDateAsync(): Promise { + const rushAlertsState: IRushAlertsState | undefined = await this._loadRushAlertsStateAsync(); - if (!rushAlertsData.lastUpdateTime) { + if (rushAlertsState === undefined || !rushAlertsState.lastUpdateTime) { return false; } const currentTime: Date = new Date(); - const lastUpdateTime: Date = new Date(rushAlertsData.lastUpdateTime); + const lastUpdateTime: Date = new Date(rushAlertsState.lastUpdateTime); const hours: number = (Number(currentTime) - Number(lastUpdateTime)) / (1000 * 60 * 60); @@ -70,7 +77,7 @@ export class RushAlerts { } public async retrieveAlertsAsync(): Promise { - const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/${RushAlerts.__rushAlertsConfigFileName}`; + const rushAlertsConfigFilePath: string = `${this._rushConfiguration.commonRushConfigFolder}/${RushConstants.rushAlertsConfigFilename}`; if (await FileSystem.existsAsync(rushAlertsConfigFilePath)) { const rushAlertsConfig: IRushAlertsConfig = JsonFile.loadAndValidate( @@ -95,16 +102,15 @@ export class RushAlerts { } public async printAlertsAsync(): Promise { - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; + const rushAlertsState: IRushAlertsState | undefined = await this._loadRushAlertsStateAsync(); - if (await FileSystem.existsAsync(rushAlertsStateFilePath)) { - const rushAlertsData: IRushAlertsState = await JsonFile.loadAsync(rushAlertsStateFilePath, { - jsonSyntax: JsonSyntax.Strict - }); - if (rushAlertsData?.alerts.length !== 0) { - for (const alert of rushAlertsData.alerts) { - this._printMessageInBoxStyle(alert); - } + if (!rushAlertsState) { + return; + } + + if (rushAlertsState?.alerts.length !== 0) { + for (const alert of rushAlertsState.alerts) { + this._printMessageInBoxStyle(alert); } } } @@ -134,16 +140,64 @@ export class RushAlerts { } } - if (alert.conditionScript) { - const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/${alert.conditionScript}`; + const conditionScript: string | undefined = alert.conditionScript; + if (conditionScript) { + // "(OPTIONAL) The filename of a script that determines whether this alert can be shown, + // found in the "common/config/rush/alert-scripts" folder." ... "To ensure up-to-date alerts, Rush + // may fetch and checkout the "common/config/rush-alerts" folder in an unpredictable temporary + // path. Therefore, your script should avoid importing dependencies from outside its folder, + // generally be kept as simple and reliable and quick as possible." + if (conditionScript.indexOf('/') >= 0 || conditionScript.indexOf('\\') >= 0) { + throw new Error( + `The rush-alerts.json file contains a "conditionScript" that is not inside the "alert-scripts" folder: ` + + JSON.stringify(conditionScript) + ); + } + const conditionScriptPath: string = `${this._rushConfiguration.rushJsonFolder}/common/config/rush/alert-scripts/${conditionScript}`; if (!(await FileSystem.existsAsync(conditionScriptPath))) { - this._terminal.writeDebugLine(`${conditionScriptPath} does not exist`); - return false; + throw new Error( + 'The "conditionScript" field in rush-alerts.json refers to a nonexistent file:\n' + + conditionScriptPath + ); } - if (!(await require(`${this._rushConfiguration.rushJsonFolder}/${alert.conditionScript}`))()) { - return false; + this._terminal.writeDebugLine(`Invoking condition script "${conditionScript}" from rush-alerts.json`); + const startTimemark: number = performance.now(); + + interface IAlertsConditionScriptModule { + canShowAlert(): boolean; + } + + let conditionScriptModule: IAlertsConditionScriptModule; + try { + conditionScriptModule = require(conditionScriptPath); + } catch (e) { + throw new Error( + `Error loading condition script "${conditionScript}" from rush-alerts.json:\n${e.toString()}` + ); + } + + const oldCwd: string = process.cwd(); + + let conditionResult: boolean; + try { + // "Rush will invoke this script with the working directory set to the monorepo root folder, + // with no guarantee that `rush install` has been run." + process.chdir(this._rushConfiguration.rushJsonFolder); + conditionResult = conditionScriptModule.canShowAlert(); + } catch (e) { + throw new Error( + `Error invoking condition script "${conditionScript}" from rush-alerts.json:\n${e.toString()}` + ); + } finally { + process.chdir(oldCwd); } + + const totalMs: number = performance.now() - startTimemark; + this._terminal.writeDebugLine( + `Invoked conditionScript "${conditionScript}"` + + ` in ${Math.round(totalMs)} ms with result "${conditionResult}"` + ); } return true; } @@ -173,27 +227,21 @@ export class RushAlerts { } private async _writeRushAlertStateAsync(validAlerts: Array): Promise { - const rushAlertsStateFilePath: string = `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`; - if (validAlerts.length > 0) { const rushAlertsState: IRushAlertsState = { lastUpdateTime: new Date().toString(), alerts: validAlerts }; - await JsonFile.saveAsync( - rushAlertsState, - `${this._rushConfiguration.commonTempFolder}/${RushAlerts.__rushAlertsStateFileName}`, - { - ignoreUndefinedValues: true, - headerComment: 'THIS FILE IS MACHINE-GENERATED -- DO NOT MODIFY', - jsonSyntax: JsonSyntax.JsonWithComments - } - ); + await JsonFile.saveAsync(rushAlertsState, this.rushAlertsStateFilePath, { + ignoreUndefinedValues: true, + headerComment: '// THIS FILE IS MACHINE-GENERATED -- DO NOT MODIFY', + jsonSyntax: JsonSyntax.JsonWithComments + }); } else { // if no valid alerts // remove exist alerts state if exist - await FileSystem.deleteFileAsync(rushAlertsStateFilePath); + await FileSystem.deleteFileAsync(this.rushAlertsStateFilePath); } } } From 70e373994f7b8e41232c7b797d788ca1e74b13ea Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 20:30:38 -0700 Subject: [PATCH 14/21] Improve error reporting for various script mistakes --- common/reviews/api/rush-lib.api.md | 1 + libraries/rush-lib/src/utilities/RushAlerts.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index efb8f313602..969d5b1b0ae 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -1359,6 +1359,7 @@ export class RushConstants { static readonly rebuildCommandName: 'rebuild'; static readonly repoStateFilename: 'repo-state.json'; static readonly rushAlertsConfigFilename: 'rush-alerts.json'; + static readonly rushAlertsStateFilename: 'rush-alerts-state.json'; static readonly rushJsonFilename: 'rush.json'; static readonly rushLogsFolderName: 'rush-logs'; static readonly rushPackageName: '@microsoft/rush'; diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index 02863d762f9..ee255497746 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -171,9 +171,13 @@ export class RushAlerts { let conditionScriptModule: IAlertsConditionScriptModule; try { conditionScriptModule = require(conditionScriptPath); + + if (typeof conditionScriptModule.canShowAlert !== 'function') { + throw new Error('The "canShowAlert" module export is missing'); + } } catch (e) { throw new Error( - `Error loading condition script "${conditionScript}" from rush-alerts.json:\n${e.toString()}` + `Error loading condition script "${conditionScript}" from rush-alerts.json:\n${e.stack}` ); } @@ -187,7 +191,7 @@ export class RushAlerts { conditionResult = conditionScriptModule.canShowAlert(); } catch (e) { throw new Error( - `Error invoking condition script "${conditionScript}" from rush-alerts.json:\n${e.toString()}` + `Error invoking condition script "${conditionScript}" from rush-alerts.json:\n${e.stack}` ); } finally { process.chdir(oldCwd); From e830cbeaa961389da2b770e4d8683565eafb38b0 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 21:01:47 -0700 Subject: [PATCH 15/21] More debugging --- libraries/rush-lib/src/utilities/RushAlerts.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index ee255497746..ab79d5462aa 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.ts @@ -189,6 +189,10 @@ export class RushAlerts { // with no guarantee that `rush install` has been run." process.chdir(this._rushConfiguration.rushJsonFolder); conditionResult = conditionScriptModule.canShowAlert(); + + if (typeof conditionResult !== 'boolean') { + throw new Error('canShowAlert() did not return a boolean value'); + } } catch (e) { throw new Error( `Error invoking condition script "${conditionScript}" from rush-alerts.json:\n${e.stack}` @@ -202,6 +206,10 @@ export class RushAlerts { `Invoked conditionScript "${conditionScript}"` + ` in ${Math.round(totalMs)} ms with result "${conditionResult}"` ); + + if (!conditionResult) { + return false; + } } return true; } @@ -233,7 +241,7 @@ export class RushAlerts { private async _writeRushAlertStateAsync(validAlerts: Array): Promise { if (validAlerts.length > 0) { const rushAlertsState: IRushAlertsState = { - lastUpdateTime: new Date().toString(), + lastUpdateTime: new Date().toISOString(), alerts: validAlerts }; From b21b210a37b04cb187be5c5a1aedaf087d946c78 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 22:29:38 -0700 Subject: [PATCH 16/21] Add a failing test --- .../terminal/src/test/PrintUtilities.test.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/libraries/terminal/src/test/PrintUtilities.test.ts b/libraries/terminal/src/test/PrintUtilities.test.ts index f09a79b3e0a..0b58dfd1dbc 100644 --- a/libraries/terminal/src/test/PrintUtilities.test.ts +++ b/libraries/terminal/src/test/PrintUtilities.test.ts @@ -103,7 +103,7 @@ describe(PrintUtilities.name, () => { .split('[n]'); expect(outputLines).toMatchSnapshot(); - expect(outputLines[0].trim().length).toEqual(expectedWidth); + expect(outputLines.every((x) => x.length <= expectedWidth)); } const MESSAGE: string = @@ -149,5 +149,24 @@ describe(PrintUtilities.name, () => { PrintUtilities.printMessageInBox(userMessage, terminal, 50); validateOutput(50); }); + + it('word-wraps a message with a trailing fragment', () => { + const lines: string[] = PrintUtilities.wrapWordsToLines( + 'This Thursday, we will complete the Node.js version upgrade. Any pipelines that still have not upgraded will be temporarily disabled.', + 36 + ); + expect(lines).toMatchInlineSnapshot(` +Array [ + "This Thursday, we will complete the", + "Node.js version upgrade. Any", + "pipelines that still have not", + "upgraded will be temporarily disabled.", +] +`); + + for (const line of lines) { + expect(line.length).toBeLessThan(36); + } + }); }); }); From 1157c129becfe4409ab22f2eeff3b50faed54b10 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 22:41:55 -0700 Subject: [PATCH 17/21] Fix a word-wrapping edge case --- libraries/terminal/src/PrintUtilities.ts | 14 +++++++++++ .../terminal/src/test/PrintUtilities.test.ts | 5 ++-- .../__snapshots__/PrintUtilities.test.ts.snap | 24 ++++++++++++------- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/libraries/terminal/src/PrintUtilities.ts b/libraries/terminal/src/PrintUtilities.ts index 6b22e6acd32..b59ea247961 100644 --- a/libraries/terminal/src/PrintUtilities.ts +++ b/libraries/terminal/src/PrintUtilities.ts @@ -156,6 +156,20 @@ export class PrintUtilities { previousWhitespaceMatch = currentWhitespaceMatch; } + if ( + previousWhitespaceMatch && + line.length + linePrefixLength - currentLineStartIndex > maxLineLength + ) { + const whitespaceToSplitAt: RegExpExecArray = previousWhitespaceMatch; + + wrappedLines.push( + linePrefix + + lineAdditionalPrefix + + line.substring(currentLineStartIndex, whitespaceToSplitAt.index) + ); + currentLineStartIndex = whitespaceToSplitAt.index + whitespaceToSplitAt[0].length; + } + if (currentLineStartIndex < line.length) { wrappedLines.push(linePrefix + lineAdditionalPrefix + line.substring(currentLineStartIndex)); } diff --git a/libraries/terminal/src/test/PrintUtilities.test.ts b/libraries/terminal/src/test/PrintUtilities.test.ts index 0b58dfd1dbc..81fc2e51526 100644 --- a/libraries/terminal/src/test/PrintUtilities.test.ts +++ b/libraries/terminal/src/test/PrintUtilities.test.ts @@ -160,12 +160,13 @@ Array [ "This Thursday, we will complete the", "Node.js version upgrade. Any", "pipelines that still have not", - "upgraded will be temporarily disabled.", + "upgraded will be temporarily", + "disabled.", ] `); for (const line of lines) { - expect(line.length).toBeLessThan(36); + expect(line.length).toBeLessThanOrEqual(36); } }); }); diff --git a/libraries/terminal/src/test/__snapshots__/PrintUtilities.test.ts.snap b/libraries/terminal/src/test/__snapshots__/PrintUtilities.test.ts.snap index da65f854acf..511965a165b 100644 --- a/libraries/terminal/src/test/__snapshots__/PrintUtilities.test.ts.snap +++ b/libraries/terminal/src/test/__snapshots__/PrintUtilities.test.ts.snap @@ -95,13 +95,16 @@ Array [ exports[`PrintUtilities wrapWordsToLines with prefix="| " applies pre-existing indents on both margins 1`] = ` Array [ "| Lorem ipsum dolor sit amet, consectetuer", - "| adipiscing elit. Maecenas porttitor congue massa.", + "| adipiscing elit. Maecenas porttitor congue", + "| massa.", "| ", "| Lorem ipsum dolor sit amet, consectetuer", - "| adipiscing elit. Maecenas porttitor congue massa.", + "| adipiscing elit. Maecenas porttitor congue", + "| massa.", "| ", "| Lorem ipsum dolor sit amet, consectetuer", - "| adipiscing elit. Maecenas porttitor congue massa.", + "| adipiscing elit. Maecenas porttitor congue", + "| massa.", ] `; @@ -111,7 +114,8 @@ Array [ "| occurrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrred", "| while pushing commits to git remote. Please make", "| sure you have installed and enabled git lfs. The", - "| easiest way to do that is run the provided setup script:", + "| easiest way to do that is run the provided setup", + "| script:", "| ", "| common/scripts/setup.sh", "| ", @@ -166,13 +170,16 @@ Array [ exports[`PrintUtilities wrapWordsToLines with prefix="4" applies pre-existing indents on both margins 1`] = ` Array [ " Lorem ipsum dolor sit amet, consectetuer", - " adipiscing elit. Maecenas porttitor congue massa.", + " adipiscing elit. Maecenas porttitor congue", + " massa.", " ", " Lorem ipsum dolor sit amet, consectetuer", - " adipiscing elit. Maecenas porttitor congue massa.", + " adipiscing elit. Maecenas porttitor congue", + " massa.", " ", " Lorem ipsum dolor sit amet, consectetuer", - " adipiscing elit. Maecenas porttitor congue massa.", + " adipiscing elit. Maecenas porttitor congue", + " massa.", ] `; @@ -254,7 +261,8 @@ Array [ "occurrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrred", "while pushing commits to git remote. Please make", "sure you have installed and enabled git lfs. The", - "easiest way to do that is run the provided setup script:", + "easiest way to do that is run the provided setup", + "script:", "", " common/scripts/setup.sh", "", From eb7f9188ba74368b5cff1ccd07777741a80d78f5 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 23:06:32 -0700 Subject: [PATCH 18/21] Fix a bug where the "message" setting didn't accept both a string and array of strings --- .../src/schemas/rush-alerts.schema.json | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/libraries/rush-lib/src/schemas/rush-alerts.schema.json b/libraries/rush-lib/src/schemas/rush-alerts.schema.json index e36ac35832d..12cc5c4b8bf 100644 --- a/libraries/rush-lib/src/schemas/rush-alerts.schema.json +++ b/libraries/rush-lib/src/schemas/rush-alerts.schema.json @@ -11,7 +11,7 @@ }, "timezone": { - "description": "Settings such as `startTime` and `endTime` will use this timezone. \n\nIf omitted, the default timezone is UTC (`+00:00`).", + "description": "Settings such as `startTime` and `endTime` will use this timezone.\n\nIf omitted, the default timezone is UTC (`+00:00`).", "type": "string" }, "alerts": { @@ -31,34 +31,37 @@ "type": "string" }, "message": { - "description": "When the alert is displayed, this text appears in the message box. \n\nTo make the JSON file more readable, if the text is longer than one line, you can instead provide an array of strings that will be concatenated. \n\nYour text may contain newline characters, but generally this is unnecessary because word-wrapping is automatically applied.", - "items": { - "$ref": "#/definitions/IAlertMessage" - }, - "type": "array" + "description": "When the alert is displayed, this text appears in the message box.\n\nTo make the JSON file more readable, if the text is longer than one line, you can instead provide an array of strings that will be concatenated.\n\nYour text may contain newline characters, but generally this is unnecessary because word-wrapping is automatically applied.", + "oneOf": [ + { + "type": "string" + }, + { + "type": "array", + "items": { + "type": "string" + } + } + ] }, "detailsUrl": { - "description": "(OPTIONAL) To avoid spamming users, the `title` and `message` settings should be kept as concise as possible. \n\nIf you need to provide more detail, use this setting to print a hyperlink to a web page with further guidance.", + "description": "(OPTIONAL) To avoid spamming users, the `title` and `message` settings should be kept as concise as possible.\n\nIf you need to provide more detail, use this setting to print a hyperlink to a web page with further guidance.", "type": "string" }, "startTime": { - "description": "(OPTIONAL) If `startTime` is specified, then this alert will not be shown prior to that time. \n\nKeep in mind that the alert is not guaranteed to be shown at this time, or at all. Alerts are only displayed after a Rush command has triggered fetching of the latest rush-alerts.json configuration. \n\nAlso, display of alerts is throttled to avoid spamming the user with too many messages. \n\nIf you need to test your alert, set the environment variable `RUSH_ALERTS_DEBUG=1` to disable throttling. \n\nThe `startTime` should be specified as `YYYY-MM-DD HH:MM` using 24 hour time format, or else `YYYY-MM-DD` in which case the time part will be `00:00` (start of that day). The time zone is obtained from the `timezone` setting above.", + "description": "(OPTIONAL) If `startTime` is specified, then this alert will not be shown prior to that time.\n\nKeep in mind that the alert is not guaranteed to be shown at this time, or at all. Alerts are only displayed after a Rush command has triggered fetching of the latest rush-alerts.json configuration.\n\nAlso, display of alerts is throttled to avoid spamming the user with too many messages.\n\nIf you need to test your alert, set the environment variable `RUSH_ALERTS_DEBUG=1` to disable throttling.\n\nThe `startTime` should be specified as `YYYY-MM-DD HH:MM` using 24 hour time format, or else `YYYY-MM-DD` in which case the time part will be `00:00` (start of that day). The time zone is obtained from the `timezone` setting above.", "type": "string" }, "endTime": { - "description": "(OPTIONAL) This alert will not be shown if the current time is later than `endTime`. \n\nThe format is the same as `startTime`.", + "description": "(OPTIONAL) This alert will not be shown if the current time is later than `endTime`.\n\nThe format is the same as `startTime`.", "type": "string" }, "conditionScript": { - "description": "(OPTIONAL) The filename of a script that determines whether this alert can be shown, found in the 'common/config/rush/alert-scripts' folder. \n\nThe script must define a CommonJS export named `canShowAlert` that returns a boolean value, for example: \n\n`module.exports.canShowAlert = function () { // (your logic goes here) return true; }`. \n\nRush will invoke this script with the working directory set to the monorepo root folder, with no guarantee that `rush install` has been run. \n\nTo ensure up-to-date alerts, Rush may fetch and checkout the 'common/config/rush-alerts' folder in an unpredictable temporary path. Therefore, your script should avoid importing dependencies from outside its folder, generally be kept as simple and reliable and quick as possible. \n\nFor more complex conditions, we suggest to design some other process that prepares a data file or environment variable that can be cheaply checked by your condition script.", + "description": "(OPTIONAL) The filename of a script that determines whether this alert can be shown, found in the 'common/config/rush/alert-scripts' folder.\n\nThe script must define a CommonJS export named `canShowAlert` that returns a boolean value, for example:\n\n`module.exports.canShowAlert = function () { // (your logic goes here) return true; }`.\n\nRush will invoke this script with the working directory set to the monorepo root folder, with no guarantee that `rush install` has been run.\n\nTo ensure up-to-date alerts, Rush may fetch and checkout the 'common/config/rush-alerts' folder in an unpredictable temporary path. Therefore, your script should avoid importing dependencies from outside its folder, generally be kept as simple and reliable and quick as possible.\n\nFor more complex conditions, we suggest to design some other process that prepares a data file or environment variable that can be cheaply checked by your condition script.", "type": "string" } }, "required": ["title", "message"] - }, - "IAlertMessage": { - "description": "Message body", - "type": "string" } } } From 58852208368c4c4ca3405b7f43cbf9dfa751c636 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 23:06:50 -0700 Subject: [PATCH 19/21] Improve formatting of messages --- .../rush-lib/src/utilities/RushAlerts.ts | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/libraries/rush-lib/src/utilities/RushAlerts.ts b/libraries/rush-lib/src/utilities/RushAlerts.ts index ab79d5462aa..ac47145f59f 100644 --- a/libraries/rush-lib/src/utilities/RushAlerts.ts +++ b/libraries/rush-lib/src/utilities/RushAlerts.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 type { Terminal } from '@rushstack/terminal'; +import { Colorize, PrintUtilities, type Terminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../api/RushConfiguration'; import { FileSystem, JsonFile, JsonSchema, JsonSyntax } from '@rushstack/node-core-library'; import rushAlertsSchemaJson from '../schemas/rush-alerts.schema.json'; @@ -29,7 +29,7 @@ interface IRushAlertsState { } interface IRushAlertStateEntry { title: string; - message: Array; + message: Array | string; detailsUrl: string; } @@ -109,6 +109,7 @@ export class RushAlerts { } if (rushAlertsState?.alerts.length !== 0) { + this._terminal.writeLine(); for (const alert of rushAlertsState.alerts) { this._printMessageInBoxStyle(alert); } @@ -215,27 +216,46 @@ export class RushAlerts { } private _printMessageInBoxStyle(alert: IRushAlertStateEntry): void { - const messageToBePrinted: Array = []; - messageToBePrinted.push(alert.title); - messageToBePrinted.push(...alert.message); - messageToBePrinted.push(alert.detailsUrl); + const boxTitle: string = alert.title.toUpperCase(); + + const boxMessage: string = typeof alert.message === 'string' ? alert.message : alert.message.join(''); + + const boxDetails: string = alert.detailsUrl ? 'Details: ' + alert.detailsUrl : ''; - // Calculate max length for the border - const maxLength: number = messageToBePrinted.reduce((max, line) => Math.max(max, line.length), 0); + // ...minus the padding. + const PADDING: number = '╔══╗'.length; - // Add padding for border - const paddedLength: number = maxLength + 4; + // Try to make it wide enough to fit the (unwrapped) strings... + let lineLength: number = Math.max(boxTitle.length, boxMessage.length, boxDetails.length); - // Create border lines - const borderLine: string = '+'.padEnd(paddedLength - 1, '-') + '+'; + // ...but don't exceed the console width, and also keep it under 80... + lineLength = Math.min(lineLength, (PrintUtilities.getConsoleWidth() ?? 80) - PADDING, 80 - PADDING); + + // ...and the width needs to be at least 40 characters... + lineLength = Math.max(lineLength, 40 - PADDING); + + const lines: string[] = [ + ...PrintUtilities.wrapWordsToLines(boxTitle, lineLength).map((x) => + Colorize.bold(x.padEnd(lineLength)) + ), + '', + ...PrintUtilities.wrapWordsToLines(boxMessage, lineLength).map((x) => x.padEnd(lineLength)) + ]; + if (boxDetails) { + lines.push( + '', + ...PrintUtilities.wrapWordsToLines(boxDetails, lineLength).map((x) => + Colorize.cyan(x.padEnd(lineLength)) + ) + ); + } // Print the box - this._terminal.writeLine(borderLine); - messageToBePrinted.forEach((line) => { - const padding: number = maxLength - line.length; - this._terminal.writeLine(`| ${line}${' '.repeat(padding)} |`); - }); - this._terminal.writeLine(borderLine); + this._terminal.writeLine('╔═' + '═'.repeat(lineLength) + '═╗'); + for (const line of lines) { + this._terminal.writeLine(`║ ${line.padEnd(lineLength)} ║`); + } + this._terminal.writeLine('╚═' + '═'.repeat(lineLength) + '═╝'); } private async _writeRushAlertStateAsync(validAlerts: Array): Promise { From a9074defc57d14b7f597f95f42334a6ecc41adbc Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 23:08:23 -0700 Subject: [PATCH 20/21] Improve changelog wording and prepare to publish a MINOR release of Rush --- .../rush/chao-rush-notification_2024-06-23-18-32.json | 2 +- common/config/rush/version-policies.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json b/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json index 1c700cfd48c..e25997cd886 100644 --- a/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json +++ b/common/changes/@microsoft/rush/chao-rush-notification_2024-06-23-18-32.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "Prototype for Rush alerts feature", + "comment": "(EXPERIMENTAL) Initial implementation of Rush alerts feature", "type": "none" } ], diff --git a/common/config/rush/version-policies.json b/common/config/rush/version-policies.json index 60e50cc8c3a..c4b37573381 100644 --- a/common/config/rush/version-policies.json +++ b/common/config/rush/version-policies.json @@ -103,7 +103,7 @@ "policyName": "rush", "definitionName": "lockStepVersion", "version": "5.129.7", - "nextBump": "patch", + "nextBump": "minor", "mainProject": "@microsoft/rush" } ] From 4bcf85b5d68b3d930d6ed73b322572e8eb2d7606 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 16 Jul 2024 23:11:45 -0700 Subject: [PATCH 21/21] rush change --- .../chao-rush-notification_2024-07-17-06-11.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@rushstack/terminal/chao-rush-notification_2024-07-17-06-11.json diff --git a/common/changes/@rushstack/terminal/chao-rush-notification_2024-07-17-06-11.json b/common/changes/@rushstack/terminal/chao-rush-notification_2024-07-17-06-11.json new file mode 100644 index 00000000000..42871439fa2 --- /dev/null +++ b/common/changes/@rushstack/terminal/chao-rush-notification_2024-07-17-06-11.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/terminal", + "comment": "Improve the PrintUtilities API to handle an edge case when word-wrapping a final line", + "type": "patch" + } + ], + "packageName": "@rushstack/terminal" +} \ No newline at end of file