Skip to content

Commit 1adad24

Browse files
jeremyruppelclaude
andcommitted
refactor: simplify shimExecFile by removing type casts and promisify handling
Use Reflect.apply and a generic type parameter instead of `as unknown as` chains. Replace promisify usage in tests with plain Promise callbacks, which eliminates the need for the promisify custom symbol wrapping entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0121302 commit 1adad24

File tree

2 files changed

+34
-44
lines changed

2 files changed

+34
-44
lines changed

test/utils/platform.test.ts

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import * as cp from "node:child_process";
22
import fs from "node:fs/promises";
33
import os from "node:os";
44
import path from "node:path";
5-
import { promisify } from "node:util";
6-
import { beforeAll, describe, expect, it } from "vitest";
5+
import {beforeAll, describe, expect, it} from "vitest";
76

87
import {
98
expectPathsEqual,
@@ -95,8 +94,8 @@ describe("platform utils", () => {
9594
const tmp = path.join(os.tmpdir(), "vscode-coder-tests-platform");
9695

9796
beforeAll(async () => {
98-
await fs.rm(tmp, { recursive: true, force: true });
99-
await fs.mkdir(tmp, { recursive: true });
97+
await fs.rm(tmp, {recursive: true, force: true});
98+
await fs.mkdir(tmp, {recursive: true});
10099
});
101100

102101
it("writes a .js file and returns its path", async () => {
@@ -115,11 +114,10 @@ describe("platform utils", () => {
115114
describe("shimExecFile", () => {
116115
const tmp = path.join(os.tmpdir(), "vscode-coder-tests-shim");
117116
const mod = shimExecFile(cp);
118-
const shimmedExecFile = promisify(mod.execFile);
119117

120118
beforeAll(async () => {
121-
await fs.rm(tmp, { recursive: true, force: true });
122-
await fs.mkdir(tmp, { recursive: true });
119+
await fs.rm(tmp, {recursive: true, force: true});
120+
await fs.mkdir(tmp, {recursive: true});
123121
});
124122

125123
it("runs .js files through node", async () => {
@@ -128,7 +126,11 @@ describe("platform utils", () => {
128126
"echo",
129127
'process.stdout.write("ok");',
130128
);
131-
const { stdout } = await shimmedExecFile(script);
129+
const stdout = await new Promise<string>((resolve, reject) => {
130+
mod.execFile(script, (err, out) =>
131+
err ? reject(err) : resolve(out),
132+
);
133+
});
132134
expect(stdout).toBe("ok");
133135
});
134136

@@ -138,24 +140,22 @@ describe("platform utils", () => {
138140
"echo-args",
139141
"process.stdout.write(process.argv.slice(2).join(','));",
140142
);
141-
const { stdout } = await shimmedExecFile(script, ["a", "b", "c"]);
143+
const stdout = await new Promise<string>((resolve, reject) => {
144+
mod.execFile(script, ["a", "b", "c"], (err, out) =>
145+
err ? reject(err) : resolve(out),
146+
);
147+
});
142148
expect(stdout).toBe("a,b,c");
143149
});
144150

145151
it("does not rewrite non-.js files", async () => {
146-
await expect(shimmedExecFile("/nonexistent/binary")).rejects.toThrow(
147-
"ENOENT",
148-
);
149-
});
150-
151-
it("preserves the callback form", async () => {
152-
const script = path.join(tmp, "echo.js");
153-
const stdout = await new Promise<string>((resolve, reject) => {
154-
mod.execFile(script, (err, out) =>
155-
err ? reject(new Error(err.message)) : resolve(out),
156-
);
157-
});
158-
expect(stdout).toBe("ok");
152+
await expect(
153+
new Promise((resolve, reject) => {
154+
mod.execFile("/nonexistent/binary", (err, out) =>
155+
err ? reject(err) : resolve(out),
156+
);
157+
}),
158+
).rejects.toThrow("ENOENT");
159159
});
160160

161161
it("does not touch spawn", () => {

test/utils/platform.ts

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import fs from "node:fs/promises";
22
import os from "node:os";
33
import path from "node:path";
4-
import { expect } from "vitest";
5-
6-
import type { ChildProcess } from "node:child_process";
4+
import * as cp from "node:child_process";
5+
import {expect} from "vitest";
76

87
export function isWindows(): boolean {
98
return os.platform() === "win32";
@@ -62,10 +61,10 @@ export async function writeExecutable(
6261
*/
6362
function prepend(file: string, rest: unknown[]): [string, ...unknown[]] {
6463
if (!file.endsWith(".js")) return [file, ...rest];
65-
const hasArgs = Array.isArray(rest[0]);
66-
const cliArgs = hasArgs ? (rest[0] as string[]) : [];
67-
const remaining = hasArgs ? rest.slice(1) : rest;
68-
return [process.execPath, [file, ...cliArgs], ...remaining];
64+
if (Array.isArray(rest[0])) {
65+
return [process.execPath, [file, ...rest[0]], ...rest.slice(1)];
66+
}
67+
return [process.execPath, [file], ...rest];
6968
}
7069

7170
/**
@@ -79,23 +78,14 @@ function prepend(file: string, rest: unknown[]): [string, ...unknown[]] {
7978
* });
8079
* ```
8180
*/
82-
type ChildProcessModule = typeof import("node:child_process");
83-
type Callable = (...args: unknown[]) => unknown;
84-
85-
export function shimExecFile(mod: ChildProcessModule): ChildProcessModule {
86-
const { execFile: original, ...rest } = mod;
87-
88-
const execFile = (file: string, ...args: unknown[]): ChildProcess =>
89-
(original as unknown as Callable)(...prepend(file, args)) as ChildProcess;
81+
export function shimExecFile<M extends {execFile: Function}>(mod: M): M {
82+
const {execFile: original} = mod;
9083

91-
const sym = Symbol.for("nodejs.util.promisify.custom");
92-
const originalCustom = (original as unknown as Record<symbol, Callable>)[sym];
93-
(execFile as unknown as Record<symbol, unknown>)[sym] = (
94-
file: string,
95-
...args: unknown[]
96-
): unknown => originalCustom(...prepend(file, args));
84+
function execFile(file: string, ...rest: unknown[]): cp.ChildProcess {
85+
return Reflect.apply(original, undefined, prepend(file, rest));
86+
}
9787

98-
return { ...rest, execFile } as ChildProcessModule;
88+
return Object.assign({}, mod, {execFile});
9989
}
10090

10191
export function expectPathsEqual(actual: string, expected: string) {

0 commit comments

Comments
 (0)