Align `FORCE_COLOR=0` with Node.js meaning

[nicolo-ribaudo]

Node.js allows disabling color by passing FORCE_COLOR=0 (https://nodejs.org/api/tty.html#writestreamgetcolordepthenv). Would you be open to a PR checking if FORCE_COLOR is 0?

For context, we just migrated Babel from Chalk to picocolors and found this difference. For now we are going to roll our own color detection logic to preserve the Node.js-like behavior.

[alexeyraspopov]

I'm considering switch to https://nodejs.org/api/tty.html#writestreamhascolorscount-env that seems to be available since Node.js v10.16. I will try to find some tests to see how it works with FORCE_COLOR variable and will make sure too keep this behavior in mind. There's probably a small patch I can make in the meantime for v1 to avoid breaking changes.

[nicolo-ribaudo]

For what is worth, I tried running a totally unscientific poll on Twitter and it seems like people are split on the behavior of FORCE_COLOR=0. While I still think that it's good for Node.js libraries to align with Node.js, changing what FORCE_COLOR=0 does should probably be considered a breaking change.

[lukeed]

As per https://no-color.org/ and https://force-color.org/, if they are set with any non-empty string, they should be considered truthy.

So its own, that means this line in picocolors is wrong:

"NO_COLOR" in env

because NO_COLOR= node ... will trigger that condition, but still be a "" value.

Node added its own flair to FORCE_COLOR (linked above) to say that FORCE_COLOR=0 is a negation.

Every other color library in the npm ecosystem follows this pattern. It's simplified here if you want it @alexeyraspopov.

I only arrived on this issue because it's linked in the @babel/highlight source, and I can't imagine any other reason that library would be loading picocolors AND chalk@2.4.1 (💀): https://github.com/babel/babel/blob/main/packages/babel-highlight/src/index.ts#L284-L299

[alexeyraspopov]

because NO_COLOR= node ... will trigger that condition, but still be a "" value.

Good point, that's something worth fixing. Not quite sure how one would end up in situation like NO_COLOR= node 😅 .

[alexeyten]

Not quite sure how one would end up in situation like NO_COLOR= node

Easy. That would be one of the thing I would try to override NO_COLOR that was set somewhere before this line. For example

export NO_COLOR=1
node aaa
node bbb
# I want only this command to use colors (and my expectation failed)
NO_COLOR= node ccc
node ddd
...

[alexeyraspopov]

Alright, I finally got a moment to fix this: https://github.com/alexeyraspopov/picocolors/pull/87. Should also cover the issue with dynamic require() calls once and for all.

[alexeyraspopov]

Version 1.1.1 is published.