Skip to content

Commit 2b7e2cc

Browse files
authored
fix(cli-repl): account for Node.js 24 multi-line REPL handling update MONGOSH-2233 (#2482)
1 parent 308a1dc commit 2b7e2cc

File tree

4 files changed

+75
-14
lines changed

4 files changed

+75
-14
lines changed

packages/cli-repl/src/async-repl.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { ReadLineOptions } from 'readline';
55
import type { ReplOptions, REPLServer } from 'repl';
66
import type { start as originalStart } from 'repl';
77
import { promisify } from 'util';
8-
import type { KeypressKey } from './repl-paste-support';
8+
import { prototypeChain, type KeypressKey } from './repl-paste-support';
99

1010
// Utility, inverse of Readonly<T>
1111
type Mutable<T> = {
@@ -407,3 +407,39 @@ function wrapPauseInput<Args extends any[], Ret>(
407407
}
408408
};
409409
}
410+
411+
// Not related to paste support, but rather for integrating with the MongoshNodeRepl's
412+
// line-by-line input handling. Calling this methods adds hooks to `repl` that are called
413+
// when the REPL is ready to evaluate further input. Eventually, like the other code
414+
// in this file, we should upstream this into Node.js core and/or evaluate the need for
415+
// it entirely.
416+
export function addReplEventForEvalReady(
417+
repl: REPLServer,
418+
before: () => boolean,
419+
after: () => void
420+
): void {
421+
const wrapMethodWithLineByLineInputNextLine = (
422+
repl: REPLServer,
423+
key: keyof REPLServer
424+
) => {
425+
if (!repl[key]) return;
426+
const originalMethod = repl[key].bind(repl);
427+
(repl as any)[key] = (...args: any[]) => {
428+
if (!before()) {
429+
return;
430+
}
431+
const result = originalMethod(...args);
432+
after();
433+
return result;
434+
};
435+
};
436+
// https://github.com/nodejs/node/blob/88f4cef8b96b2bb9d4a92f6848ce4d63a82879a8/lib/internal/readline/interface.js#L954
437+
// added in https://github.com/nodejs/node/commit/96be7836d794509dd455e66d91c2975419feed64
438+
// handles newlines inside multi-line input and replaces `.displayPrompt()` which was
439+
// previously used to print the prompt for multi-line input.
440+
const addNewLineOnTTYKey = [...prototypeChain(repl)]
441+
.flatMap((proto) => Object.getOwnPropertySymbols(proto))
442+
.find((s) => String(s).includes('(_addNewLineOnTTY)')) as keyof REPLServer;
443+
wrapMethodWithLineByLineInputNextLine(repl, 'displayPrompt');
444+
wrapMethodWithLineByLineInputNextLine(repl, addNewLineOnTTYKey);
445+
}

packages/cli-repl/src/mongosh-repl.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -521,15 +521,11 @@ class MongoshNodeRepl implements EvaluationListener {
521521
// This is used below for multiline history manipulation.
522522
let originalHistory: string[] | null = null;
523523

524-
const originalDisplayPrompt = repl.displayPrompt.bind(repl);
525-
526-
repl.displayPrompt = (...args: any[]) => {
527-
if (!this.started) {
528-
return;
529-
}
530-
originalDisplayPrompt(...args);
531-
this.lineByLineInput.nextLine();
532-
};
524+
asyncRepl.addReplEventForEvalReady(
525+
repl,
526+
() => !!this.started,
527+
() => this.lineByLineInput.nextLine()
528+
);
533529

534530
if (repl.commands.editor) {
535531
const originalEditorAction = repl.commands.editor.action.bind(repl);

packages/cli-repl/src/repl-paste-support.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export type KeypressKey = {
1111
code?: string;
1212
};
1313

14-
function* prototypeChain(obj: unknown): Iterable<unknown> {
14+
export function* prototypeChain(obj: unknown): Iterable<unknown> {
1515
if (!obj) return;
1616
yield obj;
1717
yield* prototypeChain(Object.getPrototypeOf(obj));

packages/cli-repl/src/smoke-tests.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ export async function runSmokeTests({
8888
tags,
8989
input,
9090
output,
91+
env,
9192
testArgs,
9293
includeStderr,
9394
exitCode,
@@ -120,6 +121,19 @@ export async function runSmokeTests({
120121
exitCode: 0,
121122
perfTestIterations: 0,
122123
},
124+
{
125+
// Regression test for MONGOSH-2233, included here because multiline support is a bit
126+
// more fragile when it comes to newer Node.js releases and these are the only tests
127+
// that run as part of the homebrew setup.
128+
name: 'print_multiline_terminal',
129+
input: ['{', 'print("He" + "llo" +', '" Wor" + "ld!")', '}'],
130+
env: { MONGOSH_FORCE_TERMINAL: 'true' },
131+
output: /Hello World!/,
132+
includeStderr: false,
133+
testArgs: ['--nodb'],
134+
exitCode: 0,
135+
perfTestIterations: 0,
136+
},
123137
{
124138
name: 'eval_nodb_print_plainvm',
125139
input: '',
@@ -313,7 +327,11 @@ export async function runSmokeTests({
313327
os.tmpdir(),
314328
`mongosh_smoke_test_${name}_${Date.now()}.js`
315329
);
316-
await fs.writeFile(tmpfile, input, { mode: 0o600, flag: 'wx' });
330+
await fs.writeFile(
331+
tmpfile,
332+
Array.isArray(input) ? input.join('\n') : input,
333+
{ mode: 0o600, flag: 'wx' }
334+
);
317335
cleanup.unshift(async () => await fs.unlink(tmpfile));
318336
testArgs[index] = arg.replace('$INPUT_AS_FILE', tmpfile);
319337
actualInput = '';
@@ -326,6 +344,7 @@ export async function runSmokeTests({
326344
args: [...args, ...testArgs],
327345
input: actualInput,
328346
output,
347+
env,
329348
includeStderr,
330349
exitCode,
331350
printSuccessResults: !wantPerformanceTesting,
@@ -377,6 +396,7 @@ async function runSmokeTest({
377396
name,
378397
executable,
379398
args,
399+
env,
380400
input,
381401
output,
382402
exitCode,
@@ -386,7 +406,8 @@ async function runSmokeTest({
386406
name: string;
387407
executable: string;
388408
args: string[];
389-
input: string;
409+
env?: Record<string, string | undefined>;
410+
input: string | string[];
390411
output: RegExp;
391412
exitCode?: number;
392413
includeStderr?: boolean;
@@ -398,6 +419,7 @@ async function runSmokeTest({
398419
const { spawn } = require('child_process') as typeof import('child_process');
399420
const proc = spawn(executable, [...args], {
400421
stdio: 'pipe',
422+
env: { ...process.env, ...env },
401423
});
402424
let stdout = '';
403425
let stderr = '';
@@ -407,7 +429,14 @@ async function runSmokeTest({
407429
proc.stderr?.setEncoding('utf8').on('data', (chunk) => {
408430
stderr += chunk;
409431
});
410-
proc.stdin!.end(input);
432+
if (Array.isArray(input)) {
433+
for (const chunk of input) {
434+
proc.stdin!.write(chunk + '\n');
435+
}
436+
proc.stdin!.end();
437+
} else {
438+
proc.stdin!.end(input);
439+
}
411440
const [[actualExitCode]] = await Promise.all([
412441
once(proc, 'exit'),
413442
once(proc.stdout!, 'end'),

0 commit comments

Comments
 (0)