From 1752160934264b6bbbb5d5a5cc0dcb7d03c9d7fb Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 20 Jun 2025 18:59:53 +0200 Subject: [PATCH 1/5] fix(cli-repl): account for Node.js 24 multi-line REPL handling update MONGOSH-2233 --- packages/cli-repl/src/mongosh-repl.ts | 21 +++++++------ packages/cli-repl/src/repl-paste-support.ts | 33 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/cli-repl/src/mongosh-repl.ts b/packages/cli-repl/src/mongosh-repl.ts index afe3619ad5..405872d152 100644 --- a/packages/cli-repl/src/mongosh-repl.ts +++ b/packages/cli-repl/src/mongosh-repl.ts @@ -47,7 +47,10 @@ import type { FormatOptions } from './format-output'; import { markTime } from './startup-timing'; import type { Context } from 'vm'; import { Script, createContext, runInContext } from 'vm'; -import { installPasteSupport } from './repl-paste-support'; +import { + addReplEventForEvalReady, + installPasteSupport, +} from './repl-paste-support'; import util from 'util'; declare const __non_webpack_require__: any; @@ -260,7 +263,7 @@ class MongoshNodeRepl implements EvaluationListener { repl = asyncRepl.start({ // 'repl' is not supported in startup snapshots yet. // eslint-disable-next-line @typescript-eslint/no-var-requires - start: require('pretty-repl').start, + //start: require('pretty-repl').start, input: this.lineByLineInput, output: this.output, prompt: '', @@ -521,15 +524,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(); - }; + addReplEventForEvalReady( + repl, + () => !!this.started, + () => this.lineByLineInput.nextLine() + ); if (repl.commands.editor) { const originalEditorAction = repl.commands.editor.action.bind(repl); diff --git a/packages/cli-repl/src/repl-paste-support.ts b/packages/cli-repl/src/repl-paste-support.ts index e7d6ebd0b2..ec22210b80 100644 --- a/packages/cli-repl/src/repl-paste-support.ts +++ b/packages/cli-repl/src/repl-paste-support.ts @@ -65,3 +65,36 @@ export function installPasteSupport(repl: REPLServer): string { }); return onEnd; } + +// Not related to paste support, but rather for integrating with the MongoshNodeRepl's +// line-by-line input handling. +export function addReplEventForEvalReady( + repl: REPLServer, + before: () => boolean, + after: () => void +): void { + const wrapMethodWithLineByLineInputNextLine = ( + repl: REPLServer, + key: keyof REPLServer + ) => { + const originalMethod = repl[key].bind(repl); + if (!originalMethod) return; + (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); +} From b69fba2e0fdc61a3c007f8df57d7fd989513adac Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 23 Jun 2025 11:42:10 +0200 Subject: [PATCH 2/5] fixup: ... --- packages/cli-repl/src/repl-paste-support.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-repl/src/repl-paste-support.ts b/packages/cli-repl/src/repl-paste-support.ts index ec22210b80..09a3c605c3 100644 --- a/packages/cli-repl/src/repl-paste-support.ts +++ b/packages/cli-repl/src/repl-paste-support.ts @@ -77,8 +77,8 @@ export function addReplEventForEvalReady( repl: REPLServer, key: keyof REPLServer ) => { + if (!repl[key]) return; const originalMethod = repl[key].bind(repl); - if (!originalMethod) return; (repl as any)[key] = (...args: any[]) => { if (!before()) { return; From 52ac329871d7df77195db5072c53a54a53e2f1f0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 23 Jun 2025 14:42:11 +0200 Subject: [PATCH 3/5] fixup: restructure, add comments --- packages/cli-repl/src/async-repl.ts | 38 ++++++++++++++++++++- packages/cli-repl/src/mongosh-repl.ts | 7 ++-- packages/cli-repl/src/repl-paste-support.ts | 35 +------------------ 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/cli-repl/src/async-repl.ts b/packages/cli-repl/src/async-repl.ts index b9ee863afa..d336ac6fa3 100644 --- a/packages/cli-repl/src/async-repl.ts +++ b/packages/cli-repl/src/async-repl.ts @@ -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 type Mutable = { @@ -407,3 +407,39 @@ function wrapPauseInput( } }; } + +// 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); +} diff --git a/packages/cli-repl/src/mongosh-repl.ts b/packages/cli-repl/src/mongosh-repl.ts index 405872d152..5eb78c271b 100644 --- a/packages/cli-repl/src/mongosh-repl.ts +++ b/packages/cli-repl/src/mongosh-repl.ts @@ -47,10 +47,7 @@ import type { FormatOptions } from './format-output'; import { markTime } from './startup-timing'; import type { Context } from 'vm'; import { Script, createContext, runInContext } from 'vm'; -import { - addReplEventForEvalReady, - installPasteSupport, -} from './repl-paste-support'; +import { installPasteSupport } from './repl-paste-support'; import util from 'util'; declare const __non_webpack_require__: any; @@ -524,7 +521,7 @@ class MongoshNodeRepl implements EvaluationListener { // This is used below for multiline history manipulation. let originalHistory: string[] | null = null; - addReplEventForEvalReady( + asyncRepl.addReplEventForEvalReady( repl, () => !!this.started, () => this.lineByLineInput.nextLine() diff --git a/packages/cli-repl/src/repl-paste-support.ts b/packages/cli-repl/src/repl-paste-support.ts index 09a3c605c3..ba5697e4ce 100644 --- a/packages/cli-repl/src/repl-paste-support.ts +++ b/packages/cli-repl/src/repl-paste-support.ts @@ -11,7 +11,7 @@ export type KeypressKey = { code?: string; }; -function* prototypeChain(obj: unknown): Iterable { +export function* prototypeChain(obj: unknown): Iterable { if (!obj) return; yield obj; yield* prototypeChain(Object.getPrototypeOf(obj)); @@ -65,36 +65,3 @@ export function installPasteSupport(repl: REPLServer): string { }); return onEnd; } - -// Not related to paste support, but rather for integrating with the MongoshNodeRepl's -// line-by-line input handling. -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); -} From 2454beb48b0975b855ca54872488a8adc8ee7d7f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 23 Jun 2025 14:50:36 +0200 Subject: [PATCH 4/5] fixup: test --- packages/cli-repl/src/smoke-tests.ts | 35 +++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/cli-repl/src/smoke-tests.ts b/packages/cli-repl/src/smoke-tests.ts index 15d9e21dd7..399c86fe3b 100644 --- a/packages/cli-repl/src/smoke-tests.ts +++ b/packages/cli-repl/src/smoke-tests.ts @@ -88,6 +88,7 @@ export async function runSmokeTests({ tags, input, output, + env, testArgs, includeStderr, exitCode, @@ -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. + 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: '', @@ -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 = ''; @@ -326,6 +344,7 @@ export async function runSmokeTests({ args: [...args, ...testArgs], input: actualInput, output, + env, includeStderr, exitCode, printSuccessResults: !wantPerformanceTesting, @@ -377,6 +396,7 @@ async function runSmokeTest({ name, executable, args, + env, input, output, exitCode, @@ -386,7 +406,8 @@ async function runSmokeTest({ name: string; executable: string; args: string[]; - input: string; + env?: Record; + input: string | string[]; output: RegExp; exitCode?: number; includeStderr?: boolean; @@ -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 = ''; @@ -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'), From b579b70a6ca4301e75aed655cd91755c6ee08425 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 23 Jun 2025 15:09:20 +0200 Subject: [PATCH 5/5] fixup: duh --- packages/cli-repl/src/mongosh-repl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-repl/src/mongosh-repl.ts b/packages/cli-repl/src/mongosh-repl.ts index 5eb78c271b..0061bd83c5 100644 --- a/packages/cli-repl/src/mongosh-repl.ts +++ b/packages/cli-repl/src/mongosh-repl.ts @@ -260,7 +260,7 @@ class MongoshNodeRepl implements EvaluationListener { repl = asyncRepl.start({ // 'repl' is not supported in startup snapshots yet. // eslint-disable-next-line @typescript-eslint/no-var-requires - //start: require('pretty-repl').start, + start: require('pretty-repl').start, input: this.lineByLineInput, output: this.output, prompt: '',