Skip to content

Commit 03af7f2

Browse files
authored
Fix an issue with the return type of Executable.waitForExitAsync. (#5417)
1 parent 0fd5a15 commit 03af7f2

8 files changed

Lines changed: 102 additions & 44 deletions

File tree

build-tests/api-extractor-test-05/dist/tsdoc-metadata.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"toolPackages": [
66
{
77
"packageName": "@microsoft/api-extractor",
8-
"packageVersion": "7.53.0"
8+
"packageVersion": "7.53.1"
99
}
1010
]
1111
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/node-core-library",
5+
"comment": "Update the return type of `Executable.waitForExitAsync` to omit `stdout` and `stderr` if an `encoding` parameter isn't passed to the options object.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@rushstack/node-core-library"
10+
}

common/reviews/api/node-core-library.api.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ export class Executable {
108108
static tryResolve(filename: string, options?: IExecutableResolveOptions): string | undefined;
109109
static waitForExitAsync(childProcess: child_process.ChildProcess, options: IWaitForExitWithStringOptions): Promise<IWaitForExitResult<string>>;
110110
static waitForExitAsync(childProcess: child_process.ChildProcess, options: IWaitForExitWithBufferOptions): Promise<IWaitForExitResult<Buffer>>;
111-
static waitForExitAsync(childProcess: child_process.ChildProcess, options?: IWaitForExitOptions): Promise<IWaitForExitResult<never>>;
111+
static waitForExitAsync(childProcess: child_process.ChildProcess, options?: IWaitForExitOptions): Promise<IWaitForExitResultWithoutOutput>;
112112
}
113113

114114
// @public
@@ -667,13 +667,17 @@ export interface IWaitForExitOptions {
667667
}
668668

669669
// @public
670-
export interface IWaitForExitResult<T extends Buffer | string | never = never> {
671-
exitCode: number | null;
672-
signal: string | null;
670+
export interface IWaitForExitResult<T extends Buffer | string = never> extends IWaitForExitResultWithoutOutput {
673671
stderr: T;
674672
stdout: T;
675673
}
676674

675+
// @public
676+
export interface IWaitForExitResultWithoutOutput {
677+
exitCode: number | null;
678+
signal: string | null;
679+
}
680+
677681
// @public
678682
export interface IWaitForExitWithBufferOptions extends IWaitForExitOptions {
679683
encoding: 'buffer';

libraries/node-core-library/src/Executable.ts

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -160,21 +160,12 @@ export interface IWaitForExitWithBufferOptions extends IWaitForExitOptions {
160160
}
161161

162162
/**
163-
* The result of running a process to completion using {@link Executable.(waitForExitAsync:3)}.
163+
* The result of running a process to completion using {@link Executable.(waitForExitAsync:3)}. This
164+
* interface does not include stdout or stderr output because an {@link IWaitForExitOptions.encoding} was not specified.
164165
*
165166
* @public
166167
*/
167-
export interface IWaitForExitResult<T extends Buffer | string | never = never> {
168-
/**
169-
* The process stdout output, if encoding was specified.
170-
*/
171-
stdout: T;
172-
173-
/**
174-
* The process stderr output, if encoding was specified.
175-
*/
176-
stderr: T;
177-
168+
export interface IWaitForExitResultWithoutOutput {
178169
/**
179170
* The process exit code. If the process was terminated, this will be null.
180171
*/
@@ -188,6 +179,25 @@ export interface IWaitForExitResult<T extends Buffer | string | never = never> {
188179
signal: string | null;
189180
}
190181

182+
/**
183+
* The result of running a process to completion using {@link Executable.(waitForExitAsync:1)},
184+
* or {@link Executable.(waitForExitAsync:2)}.
185+
*
186+
* @public
187+
*/
188+
export interface IWaitForExitResult<T extends Buffer | string = never>
189+
extends IWaitForExitResultWithoutOutput {
190+
/**
191+
* The process stdout output, if encoding was specified.
192+
*/
193+
stdout: T;
194+
195+
/**
196+
* The process stderr output, if encoding was specified.
197+
*/
198+
stderr: T;
199+
}
200+
191201
// Common environmental state used by Executable members
192202
interface IExecutableContext {
193203
currentWorkingDirectory: string;
@@ -554,12 +564,12 @@ export class Executable {
554564
public static async waitForExitAsync(
555565
childProcess: child_process.ChildProcess,
556566
options?: IWaitForExitOptions
557-
): Promise<IWaitForExitResult<never>>;
567+
): Promise<IWaitForExitResultWithoutOutput>;
558568

559-
public static async waitForExitAsync<T extends Buffer | string | never = never>(
569+
public static async waitForExitAsync<T extends Buffer | string>(
560570
childProcess: child_process.ChildProcess,
561571
options: IWaitForExitOptions = {}
562-
): Promise<IWaitForExitResult<T>> {
572+
): Promise<IWaitForExitResult<T> | IWaitForExitResultWithoutOutput> {
563573
const { throwOnNonZeroExitCode, throwOnSignal, encoding } = options;
564574
if (encoding && (!childProcess.stdout || !childProcess.stderr)) {
565575
throw new Error(
@@ -611,22 +621,31 @@ export class Executable {
611621
}
612622
);
613623

614-
let stdout: T | undefined;
615-
let stderr: T | undefined;
616-
if (encoding === 'buffer') {
617-
stdout = Buffer.concat(collectedStdout as Buffer[]) as T;
618-
stderr = Buffer.concat(collectedStderr as Buffer[]) as T;
619-
} else if (encoding !== undefined) {
620-
stdout = collectedStdout.join('') as T;
621-
stderr = collectedStderr.join('') as T;
622-
}
624+
let result: IWaitForExitResult<T> | IWaitForExitResultWithoutOutput;
625+
if (encoding) {
626+
let stdout: T | undefined;
627+
let stderr: T | undefined;
628+
629+
if (encoding === 'buffer') {
630+
stdout = Buffer.concat(collectedStdout as Buffer[]) as T;
631+
stderr = Buffer.concat(collectedStderr as Buffer[]) as T;
632+
} else if (encoding !== undefined) {
633+
stdout = collectedStdout.join('') as T;
634+
stderr = collectedStderr.join('') as T;
635+
}
623636

624-
const result: IWaitForExitResult<T> = {
625-
stdout: stdout as T,
626-
stderr: stderr as T,
627-
exitCode,
628-
signal
629-
};
637+
result = {
638+
stdout: stdout as T,
639+
stderr: stderr as T,
640+
exitCode,
641+
signal
642+
};
643+
} else {
644+
result = {
645+
exitCode,
646+
signal
647+
};
648+
}
630649

631650
return result;
632651
}

libraries/node-core-library/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export {
3232
type IWaitForExitWithBufferOptions,
3333
type IWaitForExitWithStringOptions,
3434
type IWaitForExitResult,
35+
type IWaitForExitResultWithoutOutput,
3536
type IProcessInfo,
3637
Executable
3738
} from './Executable';

libraries/node-core-library/src/test/Executable.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import {
1212
parseProcessListOutputAsync,
1313
type IProcessInfo,
1414
type IExecutableSpawnSyncOptions,
15-
type IWaitForExitResult
15+
type IWaitForExitResult,
16+
type IWaitForExitResultWithoutOutput
1617
} from '../Executable';
1718
import { FileSystem } from '../FileSystem';
1819
import { PosixModeBits } from '../PosixModeBits';
@@ -236,11 +237,11 @@ describe('Executable process tests', () => {
236237
environment,
237238
currentWorkingDirectory: executableFolder
238239
});
239-
const result: IWaitForExitResult = await Executable.waitForExitAsync(childProcess);
240+
const result: IWaitForExitResultWithoutOutput = await Executable.waitForExitAsync(childProcess);
240241
expect(result.exitCode).toEqual(0);
241242
expect(result.signal).toBeNull();
242-
expect(result.stderr).toBeUndefined();
243-
expect(result.stderr).toBeUndefined();
243+
expect('stdout' in result).toBe(false);
244+
expect('stderr' in result).toBe(false);
244245
});
245246

246247
test('Executable.runToCompletion(Executable.spawn("npm-binary-wrapper")) with buffer output', async () => {

libraries/rush-lib/src/utilities/Utilities.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import {
1616
SubprocessTerminator,
1717
Executable,
1818
type IWaitForExitResult,
19-
Async
19+
Async,
20+
type IWaitForExitResultWithoutOutput
2021
} from '@rushstack/node-core-library';
2122

2223
import type { RushConfiguration } from '../api/RushConfiguration';
@@ -161,6 +162,11 @@ export type OptionalToUndefined<T> = Omit<T, OptionalKeys<T>> & {
161162
[K in OptionalKeys<T>]-?: Exclude<T[K], undefined> | undefined;
162163
};
163164

165+
type IExecuteCommandInternalOptions = Omit<IExecuteCommandOptions, 'suppressOutput'> & {
166+
stdio: child_process.SpawnSyncOptions['stdio'];
167+
captureOutput: boolean;
168+
};
169+
164170
export class Utilities {
165171
public static syncNpmrc: typeof syncNpmrc = syncNpmrc;
166172

@@ -781,6 +787,16 @@ export class Utilities {
781787
* Executes the command with the specified command-line parameters, and waits for it to complete.
782788
* The current directory will be set to the specified workingDirectory.
783789
*/
790+
private static async _executeCommandInternalAsync(
791+
options: IExecuteCommandInternalOptions & { captureOutput: true }
792+
): Promise<IWaitForExitResult<string>>;
793+
/**
794+
* Executes the command with the specified command-line parameters, and waits for it to complete.
795+
* The current directory will be set to the specified workingDirectory. This does not capture output.
796+
*/
797+
private static async _executeCommandInternalAsync(
798+
options: IExecuteCommandInternalOptions & { captureOutput: false | undefined }
799+
): Promise<IWaitForExitResultWithoutOutput>;
784800
private static async _executeCommandInternalAsync({
785801
command,
786802
args,
@@ -791,10 +807,7 @@ export class Utilities {
791807
onStdoutStreamChunk,
792808
captureOutput,
793809
captureExitCodeAndSignal
794-
}: Omit<IExecuteCommandOptions, 'suppressOutput'> & {
795-
stdio: child_process.SpawnSyncOptions['stdio'];
796-
captureOutput: boolean;
797-
}): Promise<IWaitForExitResult> {
810+
}: IExecuteCommandInternalOptions): Promise<IWaitForExitResult<string> | IWaitForExitResultWithoutOutput> {
798811
const options: child_process.SpawnSyncOptions = {
799812
cwd: workingDirectory,
800813
shell: true,

0 commit comments

Comments
 (0)