Skip to content

fix(cli-repl): account for Node.js 24 multi-line REPL handling update MONGOSH-2233 #2482

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

Merged
merged 6 commits into from
Jun 24, 2025
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
38 changes: 37 additions & 1 deletion packages/cli-repl/src/async-repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { ReadLineOptions } from 'readline';
import type { ReplOptions, REPLServer } from 'repl';
import type { start as originalStart } from 'repl';
import { promisify } from 'util';
import type { KeypressKey } from './repl-paste-support';
import { prototypeChain, type KeypressKey } from './repl-paste-support';

// Utility, inverse of Readonly<T>
type Mutable<T> = {
Expand Down Expand Up @@ -407,3 +407,39 @@ function wrapPauseInput<Args extends any[], Ret>(
}
};
}

// Not related to paste support, but rather for integrating with the MongoshNodeRepl's
// line-by-line input handling. Calling this methods adds hooks to `repl` that are called
// when the REPL is ready to evaluate further input. Eventually, like the other code
// in this file, we should upstream this into Node.js core and/or evaluate the need for
// it entirely.
export function addReplEventForEvalReady(
repl: REPLServer,
before: () => boolean,
after: () => void
): void {
const wrapMethodWithLineByLineInputNextLine = (
repl: REPLServer,
key: keyof REPLServer
) => {
if (!repl[key]) return;
const originalMethod = repl[key].bind(repl);
(repl as any)[key] = (...args: any[]) => {
if (!before()) {
return;
}
const result = originalMethod(...args);
after();
return result;
};
};
// https://github.com/nodejs/node/blob/88f4cef8b96b2bb9d4a92f6848ce4d63a82879a8/lib/internal/readline/interface.js#L954
// added in https://github.com/nodejs/node/commit/96be7836d794509dd455e66d91c2975419feed64
// handles newlines inside multi-line input and replaces `.displayPrompt()` which was
// previously used to print the prompt for multi-line input.
const addNewLineOnTTYKey = [...prototypeChain(repl)]
.flatMap((proto) => Object.getOwnPropertySymbols(proto))
.find((s) => String(s).includes('(_addNewLineOnTTY)')) as keyof REPLServer;
wrapMethodWithLineByLineInputNextLine(repl, 'displayPrompt');
wrapMethodWithLineByLineInputNextLine(repl, addNewLineOnTTYKey);
}
14 changes: 5 additions & 9 deletions packages/cli-repl/src/mongosh-repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -521,15 +521,11 @@ class MongoshNodeRepl implements EvaluationListener {
// This is used below for multiline history manipulation.
let originalHistory: string[] | null = null;

const originalDisplayPrompt = repl.displayPrompt.bind(repl);

repl.displayPrompt = (...args: any[]) => {
if (!this.started) {
return;
}
originalDisplayPrompt(...args);
this.lineByLineInput.nextLine();
};
asyncRepl.addReplEventForEvalReady(
repl,
() => !!this.started,
() => this.lineByLineInput.nextLine()
);

if (repl.commands.editor) {
const originalEditorAction = repl.commands.editor.action.bind(repl);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-repl/src/repl-paste-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type KeypressKey = {
code?: string;
};

function* prototypeChain(obj: unknown): Iterable<unknown> {
export function* prototypeChain(obj: unknown): Iterable<unknown> {
if (!obj) return;
yield obj;
yield* prototypeChain(Object.getPrototypeOf(obj));
Expand Down
35 changes: 32 additions & 3 deletions packages/cli-repl/src/smoke-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export async function runSmokeTests({
tags,
input,
output,
env,
testArgs,
includeStderr,
exitCode,
Expand Down Expand Up @@ -120,6 +121,19 @@ export async function runSmokeTests({
exitCode: 0,
perfTestIterations: 0,
},
{
// Regression test for MONGOSH-2233, included here because multiline support is a bit
// more fragile when it comes to newer Node.js releases and these are the only tests
// that run as part of the homebrew setup.
Copy link
Collaborator Author

@addaleax addaleax Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth mentioning that our regular test suite does catch this (when run against Node.js 24, so we should add that to our CI soon too), we just don't run the relevant tests in homebrew.

name: 'print_multiline_terminal',
input: ['{', 'print("He" + "llo" +', '" Wor" + "ld!")', '}'],
env: { MONGOSH_FORCE_TERMINAL: 'true' },
output: /Hello World!/,
includeStderr: false,
testArgs: ['--nodb'],
exitCode: 0,
perfTestIterations: 0,
},
{
name: 'eval_nodb_print_plainvm',
input: '',
Expand Down Expand Up @@ -313,7 +327,11 @@ export async function runSmokeTests({
os.tmpdir(),
`mongosh_smoke_test_${name}_${Date.now()}.js`
);
await fs.writeFile(tmpfile, input, { mode: 0o600, flag: 'wx' });
await fs.writeFile(
tmpfile,
Array.isArray(input) ? input.join('\n') : input,
{ mode: 0o600, flag: 'wx' }
);
cleanup.unshift(async () => await fs.unlink(tmpfile));
testArgs[index] = arg.replace('$INPUT_AS_FILE', tmpfile);
actualInput = '';
Expand All @@ -326,6 +344,7 @@ export async function runSmokeTests({
args: [...args, ...testArgs],
input: actualInput,
output,
env,
includeStderr,
exitCode,
printSuccessResults: !wantPerformanceTesting,
Expand Down Expand Up @@ -377,6 +396,7 @@ async function runSmokeTest({
name,
executable,
args,
env,
input,
output,
exitCode,
Expand All @@ -386,7 +406,8 @@ async function runSmokeTest({
name: string;
executable: string;
args: string[];
input: string;
env?: Record<string, string | undefined>;
input: string | string[];
output: RegExp;
exitCode?: number;
includeStderr?: boolean;
Expand All @@ -398,6 +419,7 @@ async function runSmokeTest({
const { spawn } = require('child_process') as typeof import('child_process');
const proc = spawn(executable, [...args], {
stdio: 'pipe',
env: { ...process.env, ...env },
});
let stdout = '';
let stderr = '';
Expand All @@ -407,7 +429,14 @@ async function runSmokeTest({
proc.stderr?.setEncoding('utf8').on('data', (chunk) => {
stderr += chunk;
});
proc.stdin!.end(input);
if (Array.isArray(input)) {
for (const chunk of input) {
proc.stdin!.write(chunk + '\n');
}
proc.stdin!.end();
} else {
proc.stdin!.end(input);
}
const [[actualExitCode]] = await Promise.all([
once(proc, 'exit'),
once(proc.stdout!, 'end'),
Expand Down