Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(react): disable tty for next build #23013

Merged
merged 1 commit into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/generated/packages/nx/executors/run-commands.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@
"type": "boolean",
"description": "Whether arguments should be forwarded when interpolation is not present.",
"default": true
},
"tty": {
"type": "boolean",
"description": "Whether commands should be run with a tty terminal",
"hidden": true
}
},
"additionalProperties": true,
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/plugins/__snapshots__/plugin.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ exports[`@nx/next/plugin integrated projects should create nodes 1`] = `
],
"options": {
"cwd": "my-app",
"tty": false,
},
"outputs": [
"{workspaceRoot}/my-app/.next",
Expand Down Expand Up @@ -82,6 +83,7 @@ exports[`@nx/next/plugin root projects should create nodes 1`] = `
],
"options": {
"cwd": ".",
"tty": false,
},
"outputs": [
"{projectRoot}/.next",
Expand Down
5 changes: 5 additions & 0 deletions packages/next/src/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ async function getBuildTargetConfig(
inputs: getInputs(namedInputs),
outputs: [nextOutputPath, `${nextOutputPath}/!(cache)`],
};

// TODO(ndcunningham): Update this to be consider different versions of next.js which is running
// This doesn't actually need to be tty, but next.js has a bug, https://github.com/vercel/next.js/issues/62906, where it exits 0 when SIGINT is sent.
targetConfig.options.tty = false;

return targetConfig;
}

Expand Down
12 changes: 9 additions & 3 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export interface RunCommandsOptions extends Json {
__unparsed__: string[];
usePty?: boolean;
streamOutput?: boolean;
tty?: boolean;
}

const propKeys = [
Expand All @@ -77,6 +78,7 @@ const propKeys = [
'streamOutput',
'verbose',
'forwardAllArgs',
'tty',
];

export interface NormalizedRunCommandsOptions extends RunCommandsOptions {
Expand Down Expand Up @@ -151,7 +153,8 @@ async function runInParallel(
options.env ?? {},
true,
options.usePty,
options.streamOutput
options.streamOutput,
options.tty
).then((result: { success: boolean; terminalOutput: string }) => ({
result,
command: c.command,
Expand Down Expand Up @@ -269,7 +272,8 @@ async function runSerially(
options.env ?? {},
false,
options.usePty,
options.streamOutput
options.streamOutput,
options.tty
);
terminalOutput += result.terminalOutput;
if (!result.success) {
Expand Down Expand Up @@ -298,7 +302,8 @@ async function createProcess(
env: Record<string, string>,
isParallel: boolean,
usePty: boolean = true,
streamOutput: boolean = true
streamOutput: boolean = true,
tty: boolean
): Promise<{ success: boolean; terminalOutput: string }> {
env = processEnv(color, cwd, env);
// The rust runCommand is always a tty, so it will not look nice in parallel and if we need prefixes
Expand All @@ -319,6 +324,7 @@ async function createProcess(
cwd,
jsEnv: env,
quiet: !streamOutput,
tty,
});

childProcesses.add(cp);
Expand Down
16 changes: 14 additions & 2 deletions packages/nx/src/executors/run-commands/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,15 @@
},
"args": {
"oneOf": [
{ "type": "array", "items": { "type": "string" } },
{ "type": "string" }
{
"type": "array",
"items": {
"type": "string"
}
},
{
"type": "string"
}
],
"description": "Extra arguments. You can pass them as follows: nx run project:target --args='--wait=100'. You can then use {args.wait} syntax to interpolate them in the workspace config file. See example [above](#chaining-commands-interpolating-args-and-setting-the-cwd)"
},
Expand Down Expand Up @@ -140,6 +147,11 @@
"type": "boolean",
"description": "Whether arguments should be forwarded when interpolation is not present.",
"default": true
},
"tty": {
"type": "boolean",
"description": "Whether commands should be run with a tty terminal",
"hidden": true
}
},
"additionalProperties": true,
Expand Down
2 changes: 1 addition & 1 deletion packages/nx/src/native/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export class ChildProcess {
}
export class RustPseudoTerminal {
constructor()
runCommand(command: string, commandDir?: string | undefined | null, jsEnv?: Record<string, string> | undefined | null, quiet?: boolean | undefined | null): ChildProcess
runCommand(command: string, commandDir?: string | undefined | null, jsEnv?: Record<string, string> | undefined | null, quiet?: boolean | undefined | null, tty?: boolean | undefined | null): ChildProcess
/**
* This allows us to run a pseudoterminal with a fake node ipc channel
* this makes it possible to be backwards compatible with the old implementation
Expand Down
5 changes: 3 additions & 2 deletions packages/nx/src/native/pseudo_terminal/mac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ impl RustPseudoTerminal {
command_dir: Option<String>,
js_env: Option<HashMap<String, String>>,
quiet: Option<bool>,
tty: Option<bool>,
) -> napi::Result<ChildProcess> {
let pseudo_terminal = create_pseudo_terminal()?;
run_command(&pseudo_terminal, command, command_dir, js_env, quiet)
run_command(&pseudo_terminal, command, command_dir, js_env, quiet, tty)
}

/// This allows us to run a pseudoterminal with a fake node ipc channel
Expand All @@ -50,6 +51,6 @@ impl RustPseudoTerminal {
);

trace!("nx_fork command: {}", &command);
self.run_command(command, command_dir, js_env, Some(quiet))
self.run_command(command, command_dir, js_env, Some(quiet), Some(true))
}
}
12 changes: 10 additions & 2 deletions packages/nx/src/native/pseudo_terminal/non_mac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,16 @@ impl RustPseudoTerminal {
command_dir: Option<String>,
js_env: Option<HashMap<String, String>>,
quiet: Option<bool>,
tty: Option<bool>,
) -> napi::Result<ChildProcess> {
run_command(&self.pseudo_terminal, command, command_dir, js_env, quiet)
run_command(
&self.pseudo_terminal,
command,
command_dir,
js_env,
quiet,
tty,
)
}

/// This allows us to run a pseudoterminal with a fake node ipc channel
Expand All @@ -54,6 +62,6 @@ impl RustPseudoTerminal {
);

trace!("nx_fork command: {}", &command);
self.run_command(command, command_dir, js_env, Some(quiet))
self.run_command(command, command_dir, js_env, Some(quiet), Some(true))
}
}
15 changes: 10 additions & 5 deletions packages/nx/src/native/pseudo_terminal/pseudo_terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub fn create_pseudo_terminal() -> napi::Result<PseudoTerminal> {
}
});
}
// Why do we do this here when it's already done when running a command?
if std::io::stdout().is_tty() {
trace!("Enabling raw mode");
enable_raw_mode().expect("Failed to enter raw terminal mode");
Expand Down Expand Up @@ -95,7 +96,8 @@ pub fn create_pseudo_terminal() -> napi::Result<PseudoTerminal> {
printing_tx.send(()).ok();
});
if std::io::stdout().is_tty() {
disable_raw_mode().expect("Failed to enter raw terminal mode");
trace!("Disabling raw mode");
disable_raw_mode().expect("Failed to exit raw terminal mode");
}
Ok(PseudoTerminal {
quiet,
Expand All @@ -111,6 +113,7 @@ pub fn run_command(
command_dir: Option<String>,
js_env: Option<HashMap<String, String>>,
quiet: Option<bool>,
tty: Option<bool>,
) -> napi::Result<ChildProcess> {
let command_dir = get_directory(command_dir)?;

Expand All @@ -134,7 +137,7 @@ pub fn run_command(
let mut child = pair.slave.spawn_command(cmd)?;
pseudo_terminal.running.store(true, Ordering::SeqCst);
trace!("Running {}", command);
let is_tty = std::io::stdout().is_tty();
let is_tty = tty.unwrap_or_else(|| std::io::stdout().is_tty());
if is_tty {
trace!("Enabling raw mode");
enable_raw_mode().expect("Failed to enter raw terminal mode");
Expand All @@ -151,10 +154,10 @@ pub fn run_command(
trace!("{} Exited", command);
// This mitigates the issues with ConPTY on windows and makes it work.
running_clone.store(false, Ordering::SeqCst);
trace!("Waiting for printing to finish");
let timeout = 500;
let a = Instant::now();
if cfg!(windows) {
trace!("Waiting for printing to finish");
let timeout = 500;
let a = Instant::now();
loop {
if let Ok(_) = printing_rx.try_recv() {
break;
Expand All @@ -163,8 +166,10 @@ pub fn run_command(
break;
}
}
trace!("Printing finished");
}
if is_tty {
trace!("Disabling raw mode");
disable_raw_mode().expect("Failed to restore non-raw terminal");
}
exit_to_process_tx.send(exit.to_string()).ok();
Expand Down
4 changes: 3 additions & 1 deletion packages/nx/src/tasks-runner/pseudo-terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,16 @@ export class PseudoTerminal {
cwd,
jsEnv,
quiet,
tty,
}: {
cwd?: string;
jsEnv?: Record<string, string>;
quiet?: boolean;
tty?: boolean;
} = {}
) {
return new PseudoTtyProcess(
this.rustPseudoTerminal.runCommand(command, cwd, jsEnv, quiet)
this.rustPseudoTerminal.runCommand(command, cwd, jsEnv, quiet, tty)
);
}

Expand Down