From 5895deafdbe03d3107b25e9eb994fa0e2a416528 Mon Sep 17 00:00:00 2001 From: Tom Ballinger Date: Fri, 12 Jul 2024 18:58:54 -0700 Subject: [PATCH] Avoid console.error in CLI (#27657) Recent versions of Node.js [print console.error() in red](https://github.com/nodejs/node/pull/51629), changing the color of convex CLI output unintentionally. Change everywhere we `console.error` as a means to write to stderr to `process.stderr.write` calls. GitOrigin-RevId: 134e5302858c309e3a7fd9d470a685d9bf60b1df --- src/bundler/context.ts | 18 ++++++++++++------ src/bundler/index.test.ts | 6 +++--- src/cli/index.ts | 15 ++++++++++----- src/cli/lib/config.test.ts | 13 ++++++------- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/bundler/context.ts b/src/bundler/context.ts index b9a2578..1071579 100644 --- a/src/bundler/context.ts +++ b/src/bundler/context.ts @@ -2,6 +2,7 @@ import * as Sentry from "@sentry/node"; import chalk from "chalk"; import ora, { Ora } from "ora"; import { Filesystem, nodeFs } from "./fs.js"; +import { format } from "util"; // How the error should be handled when running `npx convex dev`. export type ErrorType = @@ -56,22 +57,27 @@ async function flushAndExit(exitCode: number, err?: any) { return process.exit(exitCode); } +// console.error before it started being red by default in Node v20 +function logToStderr(...args: unknown[]) { + process.stderr.write(`${format(...args)}\n`); +} + // Handles clearing spinner so that it doesn't get messed up export function logError(ctx: Context, message: string) { ctx.spinner?.clear(); - console.error(message); + logToStderr(message); } // Handles clearing spinner so that it doesn't get messed up export function logWarning(ctx: Context, message: string) { ctx.spinner?.clear(); - console.error(message); + logToStderr(message); } // Handles clearing spinner so that it doesn't get messed up export function logMessage(ctx: Context, ...logged: any) { ctx.spinner?.clear(); - console.error(...logged); + logToStderr(...logged); } // For the rare case writing output to stdout. Status and error messages @@ -105,7 +111,7 @@ export function changeSpinner(ctx: Context, message: string) { // Add newline to prevent clobbering ctx.spinner.text = message + "\n"; } else { - console.error(message); + logToStderr(message); } } @@ -114,7 +120,7 @@ export function logFailure(ctx: Context, message: string) { ctx.spinner.fail(message); ctx.spinner = undefined; } else { - console.error(`${chalk.red(`✖`)} ${message}`); + logToStderr(`${chalk.red(`✖`)} ${message}`); } } @@ -124,7 +130,7 @@ export function logFinishedStep(ctx: Context, message: string) { ctx.spinner.succeed(message); ctx.spinner = undefined; } else { - console.error(`${chalk.green(`✔`)} ${message}`); + logToStderr(`${chalk.green(`✔`)} ${message}`); } } diff --git a/src/bundler/index.test.ts b/src/bundler/index.test.ts index fc474ba..b9c6232 100644 --- a/src/bundler/index.test.ts +++ b/src/bundler/index.test.ts @@ -115,7 +115,7 @@ test("returns true when multiple imports and httpRouter is imported", async () = test("bundle warns about https.js|ts at top level", async () => { const fixtureDir = dirname + "/test_fixtures/js/project_with_https"; - const logSpy = vi.spyOn(console, "error"); + const logSpy = vi.spyOn(process.stderr, "write"); await entryPoints(oneoffContext, fixtureDir, false); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("https")); }); @@ -123,7 +123,7 @@ test("bundle warns about https.js|ts at top level", async () => { test("bundle does not warn about https.js|ts which is not at top level", async () => { const fixtureDir = dirname + "/test_fixtures/js/project_with_https_not_at_top_level"; - const logSpy = vi.spyOn(console, "error"); + const logSpy = vi.spyOn(process.stderr, "write"); await entryPoints(oneoffContext, fixtureDir, false); expect(logSpy).toHaveBeenCalledTimes(0); }); @@ -131,7 +131,7 @@ test("bundle does not warn about https.js|ts which is not at top level", async ( test("bundle does not warn about https.js|ts which does not import httpRouter", async () => { const fixtureDir = dirname + "/test_fixtures/js/project_with_https_without_router"; - const logSpy = vi.spyOn(console, "error"); + const logSpy = vi.spyOn(process.stderr, "write"); await entryPoints(oneoffContext, fixtureDir, false); expect(logSpy).toHaveBeenCalledTimes(0); }); diff --git a/src/cli/index.ts b/src/cli/index.ts index 8e72cd2..94461e7 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -1,4 +1,3 @@ -#!/usr/bin/env node /* eslint-disable no-restricted-syntax */ import { Command } from "@commander-js/extra-typings"; import { init } from "./init.js"; @@ -29,10 +28,16 @@ import { env } from "./env.js"; import { data } from "./data.js"; import inquirer from "inquirer"; import inquirerSearchList from "inquirer-search-list"; +import { format } from "util"; const MINIMUM_MAJOR_VERSION = 16; const MINIMUM_MINOR_VERSION = 15; +// console.error before it started being red by default in Node.js v20 +function logToStderr(...args: unknown[]) { + process.stderr.write(`${format(...args)}\n`); +} + async function main() { // If you want to use `@sentry/tracing` in your project directly, use a named import instead: // import * as SentryTracing from "@sentry/tracing" @@ -63,25 +68,25 @@ async function main() { (majorVersion === MINIMUM_MAJOR_VERSION && minorVersion < MINIMUM_MINOR_VERSION) ) { - console.error( + logToStderr( chalk.red( `Your Node version ${nodeVersion} is too old. Convex requires at least Node v${MINIMUM_MAJOR_VERSION}.${MINIMUM_MINOR_VERSION}`, ), ); - console.error( + logToStderr( chalk.gray( `You can use ${chalk.bold( "nvm", )} (https://github.com/nvm-sh/nvm#installing-and-updating) to manage different versions of Node.`, ), ); - console.error( + logToStderr( chalk.gray( "After installing `nvm`, install the latest version of Node with " + chalk.bold("`nvm install node`."), ), ); - console.error( + logToStderr( chalk.gray( "Then, activate the installed version in your terminal with " + chalk.bold("`nvm use`."), diff --git a/src/cli/lib/config.test.ts b/src/cli/lib/config.test.ts index d6267e2..7d6b885 100644 --- a/src/cli/lib/config.test.ts +++ b/src/cli/lib/config.test.ts @@ -11,17 +11,16 @@ test("parseProjectConfig", async () => { throw new Error(); }, }; - const consoleSpy = vi - .spyOn(global.console, "error") - .mockImplementation(() => { - // Do nothing - }); + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => { + // Do nothing + return true; + }); const assertParses = async (inp: any) => { expect(await parseProjectConfig(ctx, inp)).toEqual(inp); }; const assertParseError = async (inp: any, err: string) => { await expect(parseProjectConfig(ctx, inp)).rejects.toThrow(); - expect(consoleSpy).toHaveBeenCalledWith(err); + expect(stderrSpy).toHaveBeenCalledWith(err); }; await assertParses({ @@ -60,6 +59,6 @@ test("parseProjectConfig", async () => { functions: "functions/", authInfo: [{}], }, - "Expected `authInfo` in `convex.json` to be of type AuthInfo[]", + "Expected `authInfo` in `convex.json` to be of type AuthInfo[]\n", ); });