Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jcbhmr opened this issue Jul 18, 2023 · 8 comments
Open

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

jcbhmr opened this issue Jul 18, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@jcbhmr
Copy link

jcbhmr commented Jul 18, 2023

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

import * as tty from "tty"

tty && tty.isatty && tty.isatty(1) && env.TERM && !isDumbTerminal

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
Copy link
Owner

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

@jcbhmr
Copy link
Author

jcbhmr commented Jul 19, 2023

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
Copy link

jycouet commented Oct 12, 2023

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

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

@jorgebucaran
Copy link
Owner

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

@jycouet
Copy link

jycouet commented Oct 12, 2023

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
Copy link

iamandrewluca commented Mar 20, 2024

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
Copy link
Owner

Cool! Want to send me a PR @iamandrewluca?

iamandrewluca added a commit to iamandrewluca/colorette that referenced this issue Mar 20, 2024
@iamandrewluca
Copy link

@jorgebucaran here it is #103

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants