feat(common): do not use colors in CLI if not supported

[jonahsnider]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior?

isColorAllowed() will return true even when the terminal can't support colors.

Issue Number: N/A

What is the new behavior?

Colors will be enabled if the NO_COLOR environment variable is not an empty string and tty.WriteStream.prototype.hasColors() returns true.

Requires Node.js v10.16.0 or newer.

This means that users of the default NestJS logger can avoid having broken ANSI escape codes in their logs without any extra configuration.

Does this PR introduce a breaking change?

• Yes
• No

Other information

On the off chance someone is logging to a TTY that supports color, but tty.WriteStream.prototype.hasColors() says it doesn't, users can use the built-in FORCE_COLOR environment variable to override what is detected.

[coveralls]

Pull Request Test Coverage Report for Build 7bb12429-bf59-48c9-94c0-233fad22f796

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.002%) to 94.121%

---

Totals
Change from base Build a6e2ed5b-52e7-4512-9d4f-f0910f45e648:	0.002%
Covered Lines:	5763
Relevant Lines:	6123

---

💛 - Coveralls

[jmcdo29]

This is mostly only used by the ConsoleLogger, but there are references to it in the Injector.ts file as well. If it was just the ConsoleLogger I'd say this is okay, or even checking process.stdout.hasColors() directly instead of needing the prototype, but I'm not sure how to handle if this check is coming from the Injector.ts

Overall, I don't think this should be an issue, my only concern would be if someone is in an environment where hasColors() returns false but they want to force the use of colors.

[jonahsnider]

even checking process.stdout.hasColors() directly instead of needing the prototype

That only checks stdout and not stderr. See nodejs/node#40236 and nodejs/node#40240 for discussion on exposing a better way to access Writestream.prototype.hasColors().

but I'm not sure how to handle if this check is coming from the Injector.ts

isColorAllowed() could be refactored to allow an optional parameter of the stream that is being written to and call .hasColors() on that directly.

Overall, I don't think this should be an issue, my only concern would be if someone is in an environment where hasColors() returns false but they want to force the use of colors.

They can use the FORCE_COLOR environment variable.

[micalevisk]

cool!

can you run npm run format

---

(or just copy & paste)

would be

import { WriteStream } from 'tty';

const isColorAllowed = () =>
  !process.env.NO_COLOR && WriteStream.prototype.hasColors();

[kamilmysliwiec]

LGTM

[mohamed-badaoui]

Tips: For those who use Docker, do not forget to use the option tty in docker-compose service, otherwise the colors will not be displayed from this feature.

tty: true

[kamilmysliwiec]

I think previously colors were displayed even if tty wasn't set to true, can you confirm @mohamed-badaoui? If so, we should revert this PR

[mohamed-badaoui]

Yes before, the colors were displayed without passing tty param
I first tried to add the TTY parameter set to True to have colors back, but even with that, I now lose the interactive mode of the console.

I then tried to add this parameter to docker-compose:

stdin_open: true

But it does not work better. So I use the logger without color now :(

What I tried without success:

services:
  myservice:
    ...
    stdin_open: true # docker run -i
    tty: true        # docker run -t