feat(logger): improve automatic color support detection#3017
feat(logger): improve automatic color support detection#3017NamesMT wants to merge 2 commits intohonojs:mainfrom
Conversation
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const { process, Deno } = globalThis as any | ||
|
|
||
| // It is put as a function instead of a constant to only to run as needed in the ternary operator. |
There was a problem hiding this comment.
Revising in 2025:
isColorSupported was put here instead of flags because Cloudflare Workers in the past does include the env variables on the global process.env and we couldn't check for NO_COLOR, FORCE_COLOR.
If we're reviving this PR, we move it to flags now as Workers have supported it.
| const { process, Deno } = globalThis as any | ||
|
|
||
| // It is put as a function instead of a constant to only to run as needed in the ternary operator. | ||
| const isColorSupported = () => |
There was a problem hiding this comment.
Sorry if I'm misunderstanding. On Cloudflare Workers, this value returns from isColorSupported() is always false, right?
There was a problem hiding this comment.
I think so, unless the user sets FORCE_COLOR, anyways, let me do a refresh update and do actual test with Lambda + Workers to see if it works normally.
There was a problem hiding this comment.
What do you think of the added flags.ts, is it good to you?, or should I inline everything in the getColorEnabled?
There was a problem hiding this comment.
I think adding flags.ts is good. Let's go with it.
2 participants
The current color detection is not great and the log message in CloudWatch logs is hard to read.
The PR adds
utils/flags.tsand improvegetColorEnabled()usingThe flag have been moved to inline of the function to align with the current test cases.isColorSupportedflagThe author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the code