`vite` compatibility: Lazily evaluate `enabled` checks.

jorgebucaran / colorette

🌈Easily set your terminal text color & styles

MIT License

1.61k stars 51 forks source link

`vite` compatibility: Lazily evaluate `enabled` checks. #68

Closed LeoniePhiline closed 3 years ago

LeoniePhiline commented 3 years ago

Lazily evaluate enabled checks and do not fail if process.env is not set.

Both issues occur in vite's dev HMR mode, where plain, uncompiled JS modules are imported into the browser.

This makes using sanitize-html impossible with vite, since sanitize-html imports postcss into browserland in order to parse CSS into an AST for sanitization. postcss uses colorette, which fail in the browser without this PR.

This PR fixes https://github.com/jorgebucaran/colorette/issues/67

jorgebucaran commented 3 years ago

Could you check if this works too?

-const isCompatibleTerminal =
-  tty && tty.isatty(1) && env.TERM && env.TERM !== "dumb"

export const isColorSupported =
+  !isDisabled && (isForced || isWindows || (tty.isatty(1) && env.TERM && env.TERM !== "dumb") || isCI)

LeoniePhiline commented 3 years ago

Hi @jorgebucaran Thanks for your reply! Only now I got to trying what you propose.

Version 1.4.0 is used, so isColorSupported does not exist there. Colorette 2.0 is still very new and packages (autoprefixer, browserslist, listr2, postcss, rollup, stylelint) don't use it yet.

Would it be possible to merge the PR for v1.4.1 and look for a v2.0.x fix separately?

jorgebucaran commented 3 years ago

We can retroactively fix the 1.4.x line by publishing a new 1.4.1 as well as the 2.0.x line.

Could you confirm my suggestion fixes the problem?

jorgebucaran commented 3 years ago

https://github.com/jorgebucaran/colorette/issues/67#issuecomment-924884281 🎉