From 8cb0c7fd1cff4049d80c1c5972825436854cd106 Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 30 Sep 2025 19:49:47 +0000 Subject: [PATCH 1/5] [terminal] Support late add/remove of destinations --- .../terminal/late-split_2025-09-30-19-49.json | 10 ++ common/reviews/api/terminal.api.md | 4 +- libraries/terminal/src/SplitterTransform.ts | 46 ++++- .../src/test/SplitterTransform.test.ts | 157 ++++++++++++++++++ .../SplitterTransform.test.ts.snap | 114 +++++++++++++ 5 files changed, 324 insertions(+), 7 deletions(-) create mode 100644 common/changes/@rushstack/terminal/late-split_2025-09-30-19-49.json create mode 100644 libraries/terminal/src/test/SplitterTransform.test.ts create mode 100644 libraries/terminal/src/test/__snapshots__/SplitterTransform.test.ts.snap diff --git a/common/changes/@rushstack/terminal/late-split_2025-09-30-19-49.json b/common/changes/@rushstack/terminal/late-split_2025-09-30-19-49.json new file mode 100644 index 00000000000..2cadc51b2b8 --- /dev/null +++ b/common/changes/@rushstack/terminal/late-split_2025-09-30-19-49.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/terminal", + "comment": "Update API contract for `SplitterTransform` to support adding and removing destinations after creation.", + "type": "minor" + } + ], + "packageName": "@rushstack/terminal" +} \ No newline at end of file diff --git a/common/reviews/api/terminal.api.md b/common/reviews/api/terminal.api.md index 01a53e06867..97e4bc431e6 100644 --- a/common/reviews/api/terminal.api.md +++ b/common/reviews/api/terminal.api.md @@ -323,12 +323,14 @@ export class RemoveColorsTextRewriter extends TextRewriter { // @public export class SplitterTransform extends TerminalWritable { constructor(options: ISplitterTransformOptions); + addDestination(destination: TerminalWritable): void; // (undocumented) - readonly destinations: ReadonlyArray; + get destinations(): ReadonlyArray; // (undocumented) protected onClose(): void; // (undocumented) protected onWriteChunk(chunk: ITerminalChunk): void; + removeDestination(destination: TerminalWritable, close?: boolean): boolean; } // @beta diff --git a/libraries/terminal/src/SplitterTransform.ts b/libraries/terminal/src/SplitterTransform.ts index 69cfb49cc35..d3c83b236de 100644 --- a/libraries/terminal/src/SplitterTransform.ts +++ b/libraries/terminal/src/SplitterTransform.ts @@ -11,9 +11,9 @@ import type { ITerminalChunk } from './ITerminalChunk'; */ export interface ISplitterTransformOptions extends ITerminalWritableOptions { /** - * Each input chunk will be passed to each destination in the array. + * Each input chunk will be passed to each destination in the iterable. */ - destinations: TerminalWritable[]; + destinations: Iterable; } /** @@ -29,15 +29,47 @@ export interface ISplitterTransformOptions extends ITerminalWritableOptions { * @public */ export class SplitterTransform extends TerminalWritable { - public readonly destinations: ReadonlyArray; + private readonly _destinations: Set; public constructor(options: ISplitterTransformOptions) { super(); - this.destinations = [...options.destinations]; + this._destinations = new Set(options.destinations); + } + + public get destinations(): ReadonlyArray { + return [...this._destinations]; + } + + /** + * Adds a destination to the set of destinations. Duplicates are ignored. + * Only new chunks received after the destination is added will be sent to it. + * @param destination - The destination to add. + */ + public addDestination(destination: TerminalWritable): void { + this._destinations.add(destination); + } + + /** + * Removes a destination from the set of destinations. It will no longer receive chunks, and will be closed, unless + * `destination.preventAutoclose` is set to `true`. + * @param destination - The destination to remove. + * @param close - If `true` (default), the destination will be closed when removed, unless `destination.preventAutoclose` is set to `true`. + * @returns `true` if the destination was removed, `false` if it was not found. + * @remarks + * If the destination is not found, it will not be closed. + */ + public removeDestination(destination: TerminalWritable, close: boolean = true): boolean { + if (this._destinations.delete(destination)) { + if (close && !destination.preventAutoclose) { + destination.close(); + } + return true; + } + return false; } protected onWriteChunk(chunk: ITerminalChunk): void { - for (const destination of this.destinations) { + for (const destination of this._destinations) { destination.writeChunk(chunk); } } @@ -46,7 +78,7 @@ export class SplitterTransform extends TerminalWritable { const errors: Error[] = []; // If an exception is thrown, try to ensure that the other destinations get closed properly - for (const destination of this.destinations) { + for (const destination of this._destinations) { if (!destination.preventAutoclose) { try { destination.close(); @@ -56,6 +88,8 @@ export class SplitterTransform extends TerminalWritable { } } + this._destinations.clear(); + if (errors.length > 0) { throw errors[0]; } diff --git a/libraries/terminal/src/test/SplitterTransform.test.ts b/libraries/terminal/src/test/SplitterTransform.test.ts new file mode 100644 index 00000000000..0b0735bc3e8 --- /dev/null +++ b/libraries/terminal/src/test/SplitterTransform.test.ts @@ -0,0 +1,157 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { SplitterTransform } from '../SplitterTransform'; +import { MockWritable } from '../MockWritable'; +import { TerminalChunkKind, type ITerminalChunk } from '../ITerminalChunk'; + +// Helper to create chunks succinctly +function c(text: string, kind: TerminalChunkKind = TerminalChunkKind.Stdout): ITerminalChunk { + return { text, kind }; +} + +describe(SplitterTransform.name, () => { + it('writes chunks to all initial destinations', () => { + const a: MockWritable = new MockWritable(); + const b: MockWritable = new MockWritable(); + const splitter: SplitterTransform = new SplitterTransform({ destinations: [a, b] }); + + splitter.writeChunk(c('one ')); + splitter.writeChunk(c('two ', TerminalChunkKind.Stderr)); + splitter.writeChunk(c('three')); + splitter.close(); + + // Both received identical chunk sequences + expect(a.chunks).toEqual(b.chunks); + // And each chunk reference should be the exact same object instance across destinations + expect(a.chunks[0]).toBe(b.chunks[0]); + expect(a.chunks[1]).toBe(b.chunks[1]); + expect(a.chunks[2]).toBe(b.chunks[2]); + + expect(a.getFormattedChunks()).toMatchSnapshot(); + }); + + describe('addDestination', () => { + it('only receives subsequent chunks', () => { + const a: MockWritable = new MockWritable(); + const b: MockWritable = new MockWritable(); + const late: MockWritable = new MockWritable(); + const splitter: SplitterTransform = new SplitterTransform({ destinations: [a, b] }); + + splitter.writeChunk(c('early1 ')); + splitter.writeChunk(c('early2 ')); + + splitter.addDestination(late); + + splitter.writeChunk(c('late1 ')); + splitter.writeChunk(c('late2')); + splitter.close(); + + expect(a.getAllOutput()).toBe('early1 early2 late1 late2'); + expect(b.getAllOutput()).toBe('early1 early2 late1 late2'); + expect(late.getAllOutput()).toBe('late1 late2'); + + expect({ + a: a.getFormattedChunks(), + late: late.getFormattedChunks() + }).toMatchSnapshot(); + }); + }); + + describe('removeDestination', () => { + it('stops further writes and closes by default', () => { + class CloseTrackingWritable extends MockWritable { + public closed: boolean = false; + protected onClose(): void { + this.closed = true; + } + } + + const a: CloseTrackingWritable = new CloseTrackingWritable(); + const b: CloseTrackingWritable = new CloseTrackingWritable(); + const splitter: SplitterTransform = new SplitterTransform({ destinations: [a, b] }); + + splitter.writeChunk(c('first ')); + splitter.removeDestination(b); // default close=true + + splitter.writeChunk(c('second')); + splitter.close(); + + // b should not have received 'second' + expect(a.getAllOutput()).toBe('first second'); + expect(b.getAllOutput()).toBe('first '); + expect(b.closed).toBe(true); + expect(a.closed).toBe(true); // closed when splitter closed + + expect({ a: a.getFormattedChunks(), b: b.getFormattedChunks() }).toMatchSnapshot(); + }); + + it('with close=false keeps destination open', () => { + class CloseTrackingWritable extends MockWritable { + public closed: boolean = false; + protected onClose(): void { + this.closed = true; + } + } + + const a: CloseTrackingWritable = new CloseTrackingWritable(); + const b: CloseTrackingWritable = new CloseTrackingWritable(); + const splitter: SplitterTransform = new SplitterTransform({ destinations: [a, b] }); + + splitter.writeChunk(c('first ')); + splitter.removeDestination(b, false); // do not close + + splitter.writeChunk(c('second')); + splitter.close(); + + expect(b.closed).toBe(false); // still open since not auto-closed by splitter and removed + // Manually close to avoid resource leak semantics + b.close(); + expect(b.closed).toBe(true); + + expect({ a: a.getFormattedChunks(), b: b.getFormattedChunks() }).toMatchSnapshot(); + }); + + it('respects preventAutoclose', () => { + class CloseTrackingWritable extends MockWritable { + public closed: boolean = false; + public constructor(prevent: boolean) { + super({ preventAutoclose: prevent }); + } + protected onClose(): void { + this.closed = true; + } + } + + const a: CloseTrackingWritable = new CloseTrackingWritable(false); + const b: CloseTrackingWritable = new CloseTrackingWritable(true); // preventAutoclose + const splitter: SplitterTransform = new SplitterTransform({ destinations: [a, b] }); + + splitter.writeChunk(c('hello ')); + splitter.removeDestination(b); // would normally close, but preventAutoclose=true + splitter.writeChunk(c('world')); + splitter.close(); + + expect(a.closed).toBe(true); + expect(b.closed).toBe(false); // not closed due to preventAutoclose + b.close(); + expect(b.closed).toBe(true); + + expect({ a: a.getFormattedChunks(), b: b.getFormattedChunks() }).toMatchSnapshot(); + }); + + it('returns false when destination missing', () => { + const a: MockWritable = new MockWritable(); + const b: MockWritable = new MockWritable(); + const splitter: SplitterTransform = new SplitterTransform({ destinations: [a] }); + + const result: boolean = splitter.removeDestination(b); // not found + expect(result).toBe(false); + + splitter.writeChunk(c('still works')); + splitter.close(); + + expect(a.getAllOutput()).toBe('still works'); + }); + }); +}); diff --git a/libraries/terminal/src/test/__snapshots__/SplitterTransform.test.ts.snap b/libraries/terminal/src/test/__snapshots__/SplitterTransform.test.ts.snap new file mode 100644 index 00000000000..6567fcd2ee1 --- /dev/null +++ b/libraries/terminal/src/test/__snapshots__/SplitterTransform.test.ts.snap @@ -0,0 +1,114 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SplitterTransform addDestination only receives subsequent chunks 1`] = ` +Object { + "a": Array [ + Object { + "kind": "O", + "text": "early1 ", + }, + Object { + "kind": "O", + "text": "early2 ", + }, + Object { + "kind": "O", + "text": "late1 ", + }, + Object { + "kind": "O", + "text": "late2", + }, + ], + "late": Array [ + Object { + "kind": "O", + "text": "late1 ", + }, + Object { + "kind": "O", + "text": "late2", + }, + ], +} +`; + +exports[`SplitterTransform removeDestination respects preventAutoclose 1`] = ` +Object { + "a": Array [ + Object { + "kind": "O", + "text": "hello ", + }, + Object { + "kind": "O", + "text": "world", + }, + ], + "b": Array [ + Object { + "kind": "O", + "text": "hello ", + }, + ], +} +`; + +exports[`SplitterTransform removeDestination stops further writes and closes by default 1`] = ` +Object { + "a": Array [ + Object { + "kind": "O", + "text": "first ", + }, + Object { + "kind": "O", + "text": "second", + }, + ], + "b": Array [ + Object { + "kind": "O", + "text": "first ", + }, + ], +} +`; + +exports[`SplitterTransform removeDestination with close=false keeps destination open 1`] = ` +Object { + "a": Array [ + Object { + "kind": "O", + "text": "first ", + }, + Object { + "kind": "O", + "text": "second", + }, + ], + "b": Array [ + Object { + "kind": "O", + "text": "first ", + }, + ], +} +`; + +exports[`SplitterTransform writes chunks to all initial destinations 1`] = ` +Array [ + Object { + "kind": "O", + "text": "one ", + }, + Object { + "kind": "E", + "text": "two ", + }, + Object { + "kind": "O", + "text": "three", + }, +] +`; From a4ee53f72de7a15bd8c6fd6123995ea7ceae43e7 Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 30 Sep 2025 19:50:00 +0000 Subject: [PATCH 2/5] Fix clean in heft-swc-test --- build-tests/heft-swc-test/config/heft.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tests/heft-swc-test/config/heft.json b/build-tests/heft-swc-test/config/heft.json index 13f0c58de15..7a3e0bf9ef3 100644 --- a/build-tests/heft-swc-test/config/heft.json +++ b/build-tests/heft-swc-test/config/heft.json @@ -4,7 +4,7 @@ // TODO: Add comments "phasesByName": { "build": { - "cleanFiles": [{ "includeGlobs": ["dist", "lib", "lib-esnext", "lib-es5", "lib-umd", "temp"] }], + "cleanFiles": [{ "includeGlobs": ["dist", "lib-commonjs", "lib-esm", "lib-es5", "temp"] }], "tasksByName": { "typescript": { From 1a78e3b29aa1c213a9183785139e0bf503f4344f Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 30 Sep 2025 20:47:21 +0000 Subject: [PATCH 3/5] Fixup api.md --- common/reviews/api/terminal.api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/reviews/api/terminal.api.md b/common/reviews/api/terminal.api.md index 97e4bc431e6..efc7a60a1bd 100644 --- a/common/reviews/api/terminal.api.md +++ b/common/reviews/api/terminal.api.md @@ -159,7 +159,7 @@ export interface IProblemCollectorOptions extends ITerminalWritableOptions { // @public export interface ISplitterTransformOptions extends ITerminalWritableOptions { - destinations: TerminalWritable[]; + destinations: Iterable; } // @beta From 0572a1163e7b08c885c7757e44f63bec78fd08f4 Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 30 Sep 2025 21:54:38 +0000 Subject: [PATCH 4/5] Make SplitterTransform.Destinations a ReadonlySet --- common/reviews/api/terminal.api.md | 2 +- libraries/terminal/src/SplitterTransform.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/reviews/api/terminal.api.md b/common/reviews/api/terminal.api.md index efc7a60a1bd..ddd03856320 100644 --- a/common/reviews/api/terminal.api.md +++ b/common/reviews/api/terminal.api.md @@ -325,7 +325,7 @@ export class SplitterTransform extends TerminalWritable { constructor(options: ISplitterTransformOptions); addDestination(destination: TerminalWritable): void; // (undocumented) - get destinations(): ReadonlyArray; + get destinations(): ReadonlySet; // (undocumented) protected onClose(): void; // (undocumented) diff --git a/libraries/terminal/src/SplitterTransform.ts b/libraries/terminal/src/SplitterTransform.ts index d3c83b236de..cce5a428ffe 100644 --- a/libraries/terminal/src/SplitterTransform.ts +++ b/libraries/terminal/src/SplitterTransform.ts @@ -36,8 +36,8 @@ export class SplitterTransform extends TerminalWritable { this._destinations = new Set(options.destinations); } - public get destinations(): ReadonlyArray { - return [...this._destinations]; + public get destinations(): ReadonlySet { + return this._destinations; } /** From 367ac0b4d99c47a8055c45fbf85a51dc83674a7c Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 30 Sep 2025 22:19:17 +0000 Subject: [PATCH 5/5] Use `.name` for describe --- libraries/terminal/src/test/SplitterTransform.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/terminal/src/test/SplitterTransform.test.ts b/libraries/terminal/src/test/SplitterTransform.test.ts index 0b0735bc3e8..046bc86203c 100644 --- a/libraries/terminal/src/test/SplitterTransform.test.ts +++ b/libraries/terminal/src/test/SplitterTransform.test.ts @@ -31,7 +31,7 @@ describe(SplitterTransform.name, () => { expect(a.getFormattedChunks()).toMatchSnapshot(); }); - describe('addDestination', () => { + describe(SplitterTransform.prototype.addDestination.name, () => { it('only receives subsequent chunks', () => { const a: MockWritable = new MockWritable(); const b: MockWritable = new MockWritable(); @@ -58,7 +58,7 @@ describe(SplitterTransform.name, () => { }); }); - describe('removeDestination', () => { + describe(SplitterTransform.prototype.removeDestination.name, () => { it('stops further writes and closes by default', () => { class CloseTrackingWritable extends MockWritable { public closed: boolean = false;