Print logs to stderr

[gonzaloriestra]

WHY are these changes introduced?

Fixes https://github.com/Shopify/cli/issues/3863

WHAT is this pull request doing?

This PR moves these types of CLI output from stdout to stderr

• debug, informational, and error messages emitted though the existing outputInfo, outputWarn, outputDebug functions
• success and completion messages emitted through outputSuccess and outputCompleted
• rendered content emitted through renderOnce

Primary, non-message output is still output to stdout, but this intent has been more explicit by calling a new outputResult function.

How to test your changes?

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

• [ ] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
• [ ] Existing analytics will cater for this addition
• [ ] PR includes analytics changes to measure impact

Checklist

• [ ] I've considered possible cross-platform impacts (Mac, Linux, Windows)
• [ ] I've considered possible documentation changes

[github-actions[bot]]

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. → If there's no activity within a week, then a bot will automatically close this. Thanks for helping to improve Shopify's dev tooling and experience.

[github-actions[bot]]

Coverage report

St.:grey_question:	Category	Percentage	Covered / Total
🟡	Statements	76.79% (-0.19% 🔻)	9655/12574
🟡	Branches	72.17% (-0.16% 🔻)	4781/6625
🟡	Functions	76.62% (-0.14% 🔻)	2494/3255
🟡	Lines	77.29% (-0.19% 🔻)	9126/11808

Show new covered files 🐣

St.:grey_question:	File	Statements	Branches	Functions	Lines
🟡	... / output.ts	77.78%	50%	75%	75%

Show files with reduced coverage 🔻

St.:grey_question:	File	Statements	Branches	Functions	Lines
🟢	... / extension-instance.ts	84.62% (-0.59% 🔻)	79.37% (-2.38% 🔻)	92.59%	85.16%
🟢	... / flow_action.ts	90% (-10% 🔻)	100%	75% (-25% 🔻)	90% (-10% 🔻)
🟢	... / bundle.ts	78.57% (-3.78% 🔻)	75% (-12.5% 🔻)	80%	84.62% (-2.88% 🔻)
🔴	... / extension.ts	56.9% (-1.44% 🔻)	57.14% (-2.86% 🔻)	50%	57.89% (-1.43% 🔻)
🟢	... / dev-session.ts	80.31% (-5.1% 🔻)	62.5% (-6.62% 🔻)	90% (-1.89% 🔻)	82.46% (-5.24% 🔻)
🟢	... / serialize-fields.ts	95% (-2.5% 🔻)	87.5% (-3.13% 🔻)	100%	97.44%
🟢	... / binaries.ts	97.3% (+1.09% 🔼)	89.66% (+2.16% 🔼)	94.44% (-0.29% 🔻)	97.3% (+1.09% 🔼)
🟡	... / build.ts	78.69% (-1% 🔻)	64.58%	78.38% (-0.57% 🔻)	77.19% (-1.14% 🔻)
🔴	... / app-management-client.ts	42.03% (-2.34% 🔻)	40.32% (-0.45% 🔻)	39.6% (-2.4% 🔻)	40.66% (-2.55% 🔻)
🔴	... / partners-client.ts	26.14% (-0.48% 🔻)	31.58%	18.33%	25.85% (-0.5% 🔻)
🟡	... / generate.ts	67.16% (-0.48% 🔻)	57.45%	50%	72.58% (-0.44% 🔻)
🟢	... / ui.tsx	84.62% (-1.1% 🔻)	68.75% (+0.33% 🔼)	77.78%	88% (-0.46% 🔻)
🟡	... / output.ts	78.9% (+0.83% 🔼)	76.67% (+0.86% 🔼)	65.85% (-0.06% 🔻)	78.85% (+0.86% 🔼)
🔴	... / system.ts	37.5% (-16.07% 🔻)	28.57% (-16.67% 🔻)	54.55% (-9.09% 🔻)	38.18% (-16.36% 🔻)

Test suite run success

2284 tests passing in 988 suites.

Report generated by 🧪jest coverage report action from 23d4b0b74cae760fc300d0bf2c835c51948c703f

[gordonhirsch]

@gonzaloriestra and @amcaplan, I have some tophatting left to do, but would appreciate some feedback on the general direction.

[gordonhirsch]

• I see some usages of consoleLog and consoleWarn outside cli-kit, but we could replace them with outputResult and outputInfo, no? We have to simplify the options to print, because there are too many and it will be confusing for someone without enough context.

I agree. Wherever we replace consoleLog with outputInfo, etc., we'll be introducing the code paths that call shouldOutput and isUnitTest/collectLog, but that seems like a good thing to me.

• I'd try to make consoleLog and friends private. Or if it's not possible, maybe a linter rule to complain if someone tries to use them and suggest outputX.

The console* functions can be optionally passed to output* as arguments, so we'd need to make changes if we want to make them private. But I wonder if we can either remove those parameters entirely or change them to an enumeration.

• I think another source of confusion is having outputInfo/renderInfo, outputWarn/renderWarn,outputSuccess/renderSuccess... What if we take the opportunity to rename the latter functions to renderInfoBox, renderWarningBox...?

I had considered something similar: renaming outputInfo, etc., to logInfo to reduce confusion vs. outputResult. In either case, it's a pretty big change and maybe should be done separately. Also wondering if there is a way we can unify the output/render functions into a single set.

[gonzaloriestra]

But I wonder if we can either remove those parameters entirely or change them to an enumeration

I think that's a good idea.

Also wondering if there is a way we can unify the output/render functions into a single set.

That could be nice as well. I agree all of them are just printing stuff, so they should be named similarly. If we just replace all the appearances of renderX to outputXBox, it should be a simple task and a big win.

[gordonhirsch]

The latest commits remove the use of console* and other functions from outside of output.ts. The console* functions have been made private to output.ts. Others can be made private too but will require some test updates to do so, I think.

It looks like the logger parameter can't be removed from the output* functions as I had hoped because it's used to pass in custom writers.

What if we take the opportunity to rename the latter functions to renderInfoBox, renderWarningBox...?

Following up on this, I'm questioning this particular terminology. Using "Box" exposes a presentation detail that maybe shouldn't be exposed. I do agree that we should do something to reduce confusion.

[github-actions[bot]]

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. → If there's no activity within a week, then a bot will automatically close this. Thanks for helping to improve Shopify's dev tooling and experience.

[gonzaloriestra]

/snapit

[github-actions[bot]]

🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

pnpm i -g @shopify/[email protected]

[!TIP] If you get an ETARGET error, install it with NPM and the flag --@shopify:registry=https://registry.npmjs.org

[!CAUTION] After installing, validate the version by running just shopify in your terminal. If the versions don't match, you might have multiple global instances installed. Use which shopify to find out which one you are running and uninstall it.

[github-actions[bot]]

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

• Some seemingly private modules might be re-exported through public modules.
• If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/private/node/output.d.ts

import { OutputMessage, LogLevel, Logger } from '../../public/node/output.js';
/**
 * Prints a log message in the console to stdout.
 *
 * @param message - The message to print.
 */
export declare function consoleLog(message: string): void;
/**
 * Prints a warning message in the console to stderr.
 *
 * @param message - The message to print.
 */
export declare function consoleWarn(message: string): void;
/**
 * Logs an unformatted message at the given log level.
 * Note: By default,  messages are sent through the standard error.
 *
 * @param content - The content to be output to the user.
 * @param logLevel - The log level associated with the message.
 * @param logger - The logging function to use to output to the user.
 */
export declare function output(content: OutputMessage, logLevel?: LogLevel, logger?: Logger): void;

Existing type declarations

packages/cli-kit/dist/private/node/ui.d.ts

@@ -6,7 +6,7 @@ interface RenderOnceOptions {
     logger?: Logger;
     renderOptions?: RenderOptions;
 }
-export declare function renderOnce(element: JSX.Element, { logLevel, logger, renderOptions }: RenderOnceOptions): string | undefined;
+export declare function renderOnce(element: JSX.Element, { logLevel, renderOptions }: RenderOnceOptions): string | undefined;
 export declare function render(element: JSX.Element, options?: RenderOptions): Promise<unknown>;
 export declare class Stdout extends EventEmitter {
     columns: number;

packages/cli-kit/dist/public/node/output.d.ts

@@ -63,9 +63,17 @@ export declare let collectedLogs: {
 export declare function collectLog(key: string, content: OutputMessage): void;
 export declare const clearCollectedLogs: () => void;
 /**
- * Ouputs information to the user.
+ * Outputs command result information to the user.
+ * Result messages don't get additional formatting.
+ * The messages are logged at info level to stdout.
+ *
+ * @param content - The content to be output to the user.
+ */
+export declare function outputResult(content: OutputMessage): void;
+/**
+ * Logs information at the info level.
  * Info messages don't get additional formatting.
- * Note: Info messages are sent through the standard output.
+ * Note: By default, info messages are sent through the standard error.
  *
  * @param content - The content to be output to the user.
  * @param logger - The logging function to use to output to the user.
@@ -74,7 +82,7 @@ export declare function outputInfo(content: OutputMessage, logger?: Logger): voi
 /**
  * Outputs a success message to the user.
  * Success messages receive a special formatting to make them stand out in the console.
- * Note: Success messages are sent through the standard output.
+ * Note: Success messages are sent through the standard error.
  *
  * @param content - The content to be output to the user.
  * @param logger - The logging function to use to output to the user.
@@ -83,25 +91,25 @@ export declare function outputSuccess(content: OutputMessage, logger?: Logger):
 /**
  * Outputs a completed message to the user.
  * Completed message receive a special formatting to make them stand out in the console.
- * Note: Completed messages are sent through the standard output.
+ * Note: Completed messages are sent through the standard error.
  *
  * @param content - The content to be output to the user.
  * @param logger - The logging function to use to output to the user.
  */
 export declare function outputCompleted(content: OutputMessage, logger?: Logger): void;
 /**
- * Ouputs debug information to the user. By default these output is hidden unless the user calls the CLI with --verbose.
+ * Logs a message at the debug level. By default these output is hidden unless the user calls the CLI with --verbose.
  * Debug messages don't get additional formatting.
- * Note: Debug messages are sent through the standard output.
+ * Note: By default, debug messages are sent through the standard error.
  *
  * @param content - The content to be output to the user.
  * @param logger - The logging function to use to output to the user.
  */
 export declare function outputDebug(content: OutputMessage, logger?: Logger): void;
 /**
- * Outputs a warning message to the user.
+ * Logs a message at the warning level.
  * Warning messages receive a special formatting to make them stand out in the console.
- * Note: Warning messages are sent through the standard output.
+ * Note: By default, warning messages are sent through the standard error.
  *
  * @param content - The content to be output to the user.
  * @param logger - The logging function to use to output to the user.
@@ -138,24 +146,6 @@ export interface OutputProcess {
      */
     action: (stdout: Writable, stderr: Writable, signal: AbortSignal) => Promise<void>;
 }
-/**
- * Prints a log message in the console.
- *
- * @param message - The message to print.
- */
-export declare function consoleLog(message: string): void;
-/**
- * Prints an error message in the console.
- *
- * @param message - The message to print.
- */
-export declare function consoleError(message: string): void;
-/**
- * Prints a warning message in the console.
- *
- * @param message - The message to print.
- */
-export declare function consoleWarn(message: string): void;
 /**
  * Writes a message to the appropiated logger.
  *

packages/cli-kit/dist/public/node/ui.d.ts

@@ -1,5 +1,4 @@
 import { FatalError as Fatal } from './error.js';
-import { Logger, LogLevel } from './output.js';
 import { ConcurrentOutputProps } from '../../private/node/ui/components/ConcurrentOutput.js';
 import { handleCtrlC, render } from '../../private/node/ui.js';
 import { AlertOptions } from '../../private/node/ui/alert.js';
@@ -370,18 +369,6 @@ export interface RenderDangerousConfirmationPromptOptions extends Omit<Dangerous
  *
  */
 export declare function renderDangerousConfirmationPrompt({ renderOptions, ...props }: RenderDangerousConfirmationPromptOptions, uiDebugOptions?: UIDebugOptions): Promise<boolean>;
-interface RenderTextOptions {
-    text: string;
-    logLevel?: LogLevel;
-    logger?: Logger;
-}
-/** Renders a text string to the console.
- * Using this function makes sure that correct spacing is applied among the various components.
- * @example
- * Hello world!
- *
- */
-export declare function renderText({ text, logLevel, logger }: RenderTextOptions): string;
 /** Waits for any key to be pressed except Ctrl+C which will terminate the process. */
 export declare const keypress: (stdin?: NodeJS.ReadStream & {
     fd: 0;