jorgebucaran / colorette

🌈Easily set your terminal text color & styles
MIT License
1.61k stars 51 forks source link

Use process.stdout.isTTY instead of node:tty #100

Open jcbhmr opened 1 year ago

jcbhmr commented 1 year ago

The preferred method of determining whether Node.js is being run within a TTY context is to check that the value of the process.stdout.isTTY property is true:

$ node -p -e "Boolean(process.stdout.isTTY)"
true
$ node -p -e "Boolean(process.stdout.isTTY)" | cat
false

https://nodejs.org/api/tty.html#tty

https://github.com/jorgebucaran/colorette/blob/20fc196d07d0f87c61e0256eadd7831c79b24108/index.js#L1

https://github.com/jorgebucaran/colorette/blob/20fc196d07d0f87c61e0256eadd7831c79b24108/index.js#L15

globalThis.process?.stdout?.isTTY && env.TERM && !isDumbTerminal 

This would also remove the dependency on the node:tty module and make it "more easier" for use in the browser. i.e. NO TRANSPILATION required, you can just import ... from "./node_modules/colorette/index.js"

jorgebucaran commented 1 year ago

What's the minimum Node version needed to run this?

jcbhmr commented 1 year ago

Definitely all LTS versions.

image

.isTTY is since v0.5.8

Cannot find exact date when process.stdout was instance of tty.WriteStream when it is a tty, but I assume very early on.

jycouet commented 1 year ago

We would be able to use it in the browser? Like this 👇

https://svelte.dev/repl/032bb6cf5c0043a093e6c2a081c1957d?version=4.2.1

jorgebucaran commented 1 year ago

Just by making this change we could use Colorette in the browser? How would that work?

jycouet commented 1 year ago

Because in most browsers, console.log('\x1b[36m%s\x1b[0m', 'I am cyan'); works.

But browser bundler have a hard time with tty. image (You can see the message in the REPL I sent upper)

iamandrewluca commented 8 months ago

Also, this would allow us to use it in Edge Runtimes.

I'm using Next.js, and it is reporting something like this on the build.

   Creating an optimized production build ...
 âš  Compiled with warnings

./node_modules/colorette/index.js
A Node.js module is loaded ('tty' at line 1) which is not supported in the Edge Runtime.
Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime

Import trace for requested module:
./node_modules/colorette/index.js
./node_modules/znv/dist/reporter.js
./node_modules/znv/dist/parse-env.js
./node_modules/znv/dist/index.js
./src/env.ts

Using the patch-package with this patch seems to work fine

patches/colorette+2.0.20.patch

diff --git a/node_modules/colorette/index.js b/node_modules/colorette/index.js
index 0d64e6b..a6ab8d1 100644
--- a/node_modules/colorette/index.js
+++ b/node_modules/colorette/index.js
@@ -1,5 +1,3 @@
-import * as tty from "tty"
-
 const {
   env = {},
   argv = [],
@@ -10,9 +8,7 @@ const isDisabled = "NO_COLOR" in env || argv.includes("--no-color")
 const isForced = "FORCE_COLOR" in env || argv.includes("--color")
 const isWindows = platform === "win32"
 const isDumbTerminal = env.TERM === "dumb"
-
-const isCompatibleTerminal =
-  tty && tty.isatty && tty.isatty(1) && env.TERM && !isDumbTerminal
+const isCompatibleTerminal = process.stdout?.isTTY && env.TERM && !isDumbTerminal

 const isCI =
   "CI" in env &&
jorgebucaran commented 8 months ago

Cool! Want to send me a PR @iamandrewluca?

iamandrewluca commented 8 months ago

@jorgebucaran here it is https://github.com/jorgebucaran/colorette/pull/103