Print logs to stderr

[gonzaloriestra]

WHY are these changes introduced?

Fixes #3863

WHAT is this pull request doing?

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]

Unused dependencies (1)

Filename	dependencies
packages/app/package.json	micromatch

Unused devDependencies (1)

Filename	devDependencies
packages/app/package.json	@types/micromatch

Unused types (1)

Filename	types
packages/app/src/cli/services/build/extension.ts	BuildFunctionExtensionOptions

[github-actions]

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

We found no new type declarations in this PR

Existing type declarations

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

@@ -141,19 +141,19 @@ export interface OutputProcess {
     action: (stdout: Writable, stderr: Writable, signal: AbortSignal) => Promise<void>;
 }
 /**
- * Prints a log message in the console.
+ * Prints a log message in the console to stdout.
  *
  * @param message - The message to print.
  */
 export declare function consoleLog(message: string): void;
 /**
- * Prints an error message in the console.
+ * Prints an error message in the console to stderr.
  *
  * @param message - The message to print.
  */
 export declare function consoleError(message: string): void;
 /**
- * Prints a warning message in the console.
+ * Prints a warning message in the console to stderr.
  *
  * @param message - The message to print.
  */

[charlespwd]

May I suggest a export function output(content: outputMessage, logger: Logger = consoleLog): void {

?

That way all the output{log,warn,error} would all go to stderr and we'd make it explicit which one goes to stdout?

STDOUT = scriptable output
STDERR = logging

[amcaplan]

console.log goes to stdout...

I think the idea is that we have several output functions based on log level, but by default all log to consoleWarn which uses console.warn which goes to stderr.

[charlespwd]

Yeah that's what I'm saying. Let's change this so that console.log is only used intentionally.

• outputInfo - STDERR loglevel info
• outputDebug - STDERR loglevel debug
• outputWarn - STDERR loglevel warn
• outputError - STDERR loglevel error
• output - STDOUT. Use only for JSON or whatever could be piped into another bash script.

[amcaplan]

I don't think we should ever log anything without specifying a log level. We can default everything to stderr and make stdout an explicit choice by specifying stdout as the logger.

[charlespwd]

idk I think output is output and logging is logging. I personally don't see a problem with these two things:

• logging that goes to STDERR and requires a loglevel
• output goes to STDOUT and doesn't require a loglevel.

STDOUT is for piping & scripts. Why would we want a loglevel in there? What am I missing?

[amcaplan]

Because, while we don't currently support it, many programs have a silent log level which doesn't print anything. And I think we should ultimately support that. Keeping everything with log levels gives us more room to fine-tune logging behavior.

[amcaplan]

Here is a good example: https://docs.npmjs.com/cli/v10/using-npm/config#loglevel

Silent is just another log level.

[charlespwd]

Wouldn't that mean that --json --silent do nothing at all? There's always &> /dev/null?

Whenever I'm scripting, I'm mostly annoyed by shit showing up in STDOUT and that's why I'd want --silent, but if it doesn't, then I don't see the point.

One of pnpm or nx is an example offender here. If I want to run the shopify theme language-server with the dev build in Shopify/cli, I can only do that by running the build node package stuff because otherwise pnpm & nx are outputting crap to STDOUT that don't fit the LSP.

Same goes for pnpm shopify theme check --json.

[charlespwd]

For that reason, whenever I want to test a Shopify/theme-tools bump in vim. I have to use a script in my $HOME/bin that is this:

#!/usr/bin/env bash

if [[ -z $DEBUG ]]; then
  node /Users/charles/src/github.com/Shopify/cli/packages/cli/bin/dev.js "$@"
else
  node --inspect-brk /Users/charles/src/github.com/Shopify/cli/packages/cli/bin/dev.js "$@"
fi

Which means I need pnpm run build any time I want to test it too.

[amcaplan]

Have you tried pnpm --silent shopify:run ...

--silent quiets pnpm

shopify:run skips the build step (you have to run pnpm shopify build first, but anyway you need to do that in your script)