Skip to content

Commit c633ca7

Browse files
ClaudeBrooooooklyn
andcommitted
Fix stdin TTY handling and add integration tests for issue #511
Co-authored-by: Brooooooklyn <3468483+Brooooooklyn@users.noreply.github.com> Agent-Logs-Url: https://github.com/oxc-project/oxc-node/sessions/6f816136-6098-46d5-8958-6181480b5826
1 parent f459893 commit c633ca7

4 files changed

Lines changed: 138 additions & 11 deletions

File tree

packages/cli/src/index.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#!/usr/bin/env node
22

3-
import { exec, execSync } from "node:child_process";
3+
import { execSync, spawn } from "node:child_process";
44
import process from "node:process";
55

66
import { Builtins, Cli, Command, Option, Usage } from "clipanion";
@@ -52,22 +52,14 @@ class MainCommand extends Command {
5252
});
5353
return;
5454
}
55-
const cp = exec(`node --enable-source-maps --import ${register} ${args}`, {
55+
const cp = spawn(`node`, [`--enable-source-maps`, `--import`, register, ...this.args], {
5656
env: process.env,
5757
cwd: process.cwd(),
58+
stdio: `inherit`,
5859
});
5960
cp.addListener(`error`, (error) => {
6061
console.error(error);
6162
});
62-
if (cp.stdin) {
63-
this.context.stdin.pipe(cp.stdin);
64-
}
65-
if (cp.stdout) {
66-
cp.stdout.pipe(this.context.stdout);
67-
}
68-
if (cp.stderr) {
69-
cp.stderr.pipe(this.context.stderr);
70-
}
7163
cp.addListener(`exit`, (code) => {
7264
process.exit(code ?? 0);
7365
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/usr/bin/env node
2+
// Test fixture that reads from stdin and echoes it back
3+
import process from "node:process";
4+
5+
let data = "";
6+
7+
process.stdin.on("data", (chunk) => {
8+
data += chunk;
9+
});
10+
11+
process.stdin.on("end", () => {
12+
console.log(data.trim());
13+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/usr/bin/env node
2+
// Test fixture to check if stdin has TTY properties
3+
import process from "node:process";
4+
5+
// Output stdin.isTTY status
6+
console.log(
7+
JSON.stringify({
8+
isTTY: process.stdin.isTTY,
9+
hasSetRawMode: typeof process.stdin.setRawMode === "function",
10+
}),
11+
);
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import test from "ava";
2+
import { spawn, spawnSync } from "node:child_process";
3+
import { fileURLToPath } from "node:url";
4+
import { promisify } from "node:util";
5+
6+
const FIXTURE_PATHS = new Map(
7+
["tty-check.ts", "stdin-echo.ts"].map((name) => [
8+
name,
9+
fileURLToPath(new URL(`./fixtures/${name}`, import.meta.url)),
10+
]),
11+
);
12+
13+
const getFixturePath = (fixtureName: string) => {
14+
const resolved = FIXTURE_PATHS.get(fixtureName);
15+
if (!resolved) {
16+
throw new Error(`Unknown fixture: ${fixtureName}`);
17+
}
18+
return resolved;
19+
};
20+
21+
const oxnodePath = fileURLToPath(new URL("../../cli/dist/index.js", import.meta.url));
22+
23+
test("CLI preserves TTY properties when stdin is a TTY", (t) => {
24+
const fixturePath = getFixturePath("tty-check.ts");
25+
26+
// Run with oxnode via spawn to inherit stdio
27+
// Note: In CI environment, stdin might not be a TTY, but we can still test
28+
// that oxnode doesn't break TTY when one is available
29+
const result = spawnSync(process.execPath, [oxnodePath, fixturePath], {
30+
encoding: "utf8",
31+
env: {
32+
...process.env,
33+
NODE_OPTIONS: undefined,
34+
},
35+
});
36+
37+
t.falsy(result.error, result.error?.message);
38+
t.is(result.status, 0, `oxnode should run successfully. stderr: ${result.stderr}`);
39+
40+
const output = JSON.parse(result.stdout.trim());
41+
42+
// In a non-TTY environment (like CI), isTTY will be undefined and omitted from JSON
43+
// In a TTY environment, isTTY should be true
44+
// The key is that we're checking it's not explicitly broken by piping
45+
if ("isTTY" in output) {
46+
t.true(output.isTTY, "when present, isTTY should be true");
47+
}
48+
// When stdin is a TTY, setRawMode should be available
49+
// When stdin is not a TTY, setRawMode might not be available
50+
t.is(typeof output.hasSetRawMode, "boolean", "hasSetRawMode should be a boolean");
51+
});
52+
53+
test("CLI properly handles stdin piping", async (t) => {
54+
const fixturePath = getFixturePath("stdin-echo.ts");
55+
const testInput = "Hello from stdin test";
56+
57+
// Spawn oxnode and pipe input to it
58+
const child = spawn(process.execPath, [oxnodePath, fixturePath], {
59+
env: {
60+
...process.env,
61+
NODE_OPTIONS: undefined,
62+
},
63+
});
64+
65+
let stdout = "";
66+
let stderr = "";
67+
68+
child.stdout?.on("data", (chunk) => {
69+
stdout += chunk;
70+
});
71+
72+
child.stderr?.on("data", (chunk) => {
73+
stderr += chunk;
74+
});
75+
76+
// Write test input to stdin and close it
77+
child.stdin?.write(testInput);
78+
child.stdin?.end();
79+
80+
// Wait for child to exit
81+
await new Promise((resolve) => {
82+
child.on("exit", resolve);
83+
});
84+
85+
t.is(stderr, "", "should not produce any errors");
86+
t.is(stdout.trim(), testInput, "should echo the input correctly");
87+
});
88+
89+
test("CLI with node directly preserves TTY (baseline comparison)", (t) => {
90+
const fixturePath = getFixturePath("tty-check.ts");
91+
92+
// Run with node directly with --import for baseline comparison
93+
const result = spawnSync(process.execPath, ["--import", "@oxc-node/core/register", fixturePath], {
94+
encoding: "utf8",
95+
env: {
96+
...process.env,
97+
NODE_OPTIONS: undefined,
98+
},
99+
});
100+
101+
t.falsy(result.error, result.error?.message);
102+
t.is(result.status, 0, "node with --import should run successfully");
103+
104+
const output = JSON.parse(result.stdout.trim());
105+
106+
// This is the baseline - both oxnode and node --import should behave the same
107+
if ("isTTY" in output) {
108+
t.true(output.isTTY, "baseline: when present, isTTY should be true");
109+
}
110+
t.is(typeof output.hasSetRawMode, "boolean", "baseline: hasSetRawMode should be a boolean");
111+
});

0 commit comments

Comments
 (0)