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.

Sign up for GitHub

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

Jump to bottom

supportsColor() should recheck process.env.FORCE_COLOR #114

Closed
jaydenseric opened this issue Dec 14, 2020 · 2 comments · Fixed by #115
Closed

supportsColor() should recheck process.env.FORCE_COLOR #114

jaydenseric opened this issue Dec 14, 2020 · 2 comments · Fixed by #115
Labels
bug help wanted

Comments

@jaydenseric
Copy link

At the moment, the supportsColor() function does not consider all the environment variables fresh each time it's called. This means changing process.env.FORCE_COLOR at runtime has no effect on the supportsColor() result.

This bug prevents us from being able to do process.env.FORCE_COLOR = '1', run a snapshot test of something that uses chalk/supports-color for formatting, then revert the FORCE_COLOR back again.

This issue probably blocks a fix for babel/babel#12442 .
@Qix-
Copy link
Member

Qix- commented Dec 14, 2020 •
edited
Loading

Are you setting that variable prior to including any modules that use supports-color? Or after?

There's always just calling supportsColor.supportsColor(process.stdout) again. That doesn't cache.

EDIT: Oh I see, we do cache that one. Agreed, we shouldn't be doing that.

@sindresorhus
Copy link
Member

PR welcome to fix this.

LKummer added a commit to LKummer/supports-color that referenced this issue Jan 17, 2021
@LKummer
Add test for chalk#114
7e59ef9 
Add test ensuring supportsColor() does not cache FORCE_COLOR environment
variable.
LKummer added a commit to LKummer/supports-color that referenced this issue Jan 17, 2021
@LKummer
Fix chalk#114
7980fc0 
Fix FORCE_COLOR environment variable being cached, trying to keep the
scope of the fix as small as possible to avoid introducing new issues.
@LKummer LKummer mentioned this issue Jan 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix check of FORCE_COLOR env variable LKummer/supports-color
3 participants
@sindresorhus @Qix- @jaydenseric