Skip to content

fix(lighthouse): hide logs timestamp when running in tty #50

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

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

erezrokah
Copy link
Contributor

See before:
https://app.netlify.com/sites/plugin-lighthouse/deploys/5f0c23c7c9f52900075bf9ca

12:06:16 PM: Mon, 13 Jul 2020 09:06:16 GMT ChromeLauncher Waiting for browser.
12:06:16 PM: Mon, 13 Jul 2020 09:06:16 GMT ChromeLauncher Waiting for browser...
12:06:16 PM: Mon, 13 Jul 2020 09:06:16 GMT ChromeLauncher Waiting for browser.....
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT ChromeLauncher Waiting for browser.......
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT ChromeLauncher Waiting for browser.......✓
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT config:warn IFrameElements gatherer requested, however no audit requires it.
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT config:warn InspectorIssues gatherer requested, however no audit requires it.
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT status Connecting to browser
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT status Resetting state with about:blank
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT status Benchmarking machine
12:06:17 PM: Mon, 13 Jul 2020 09:06:17 GMT status Initializing…

And after:
https://app.netlify.com/sites/plugin-lighthouse/deploys/5f0c4a4177fc940007d8a269

2:49:52 PM:   ChromeLauncher Waiting for browser. +0ms
2:49:52 PM:   ChromeLauncher Waiting for browser... +1ms
2:49:53 PM:   ChromeLauncher Waiting for browser..... +505ms
2:49:53 PM:   ChromeLauncher Waiting for browser.....✓ +3ms
2:49:53 PM:   config:warn IFrameElements gatherer requested, however no audit requires it. +223ms
2:49:53 PM:   config:warn InspectorIssues gatherer requested, however no audit requires it. +0ms
2:49:53 PM:   status Connecting to browser +21ms
2:49:53 PM:   status Resetting state with about:blank +15ms
2:49:53 PM:   status Benchmarking machine +54ms
2:49:53 PM:   status Initializing… +507ms

Lighthouse uses debug as a dependency which appends a timestamp when in tty:
https://github.com/visionmedia/debug/blob/80ef62a3af4df95250d77d64edfc3d0e1667e7e8/src/node.js#L174
https://github.com/visionmedia/debug/blob/80ef62a3af4df95250d77d64edfc3d0e1667e7e8/src/node.js#L151

With older versions (like the one lighthouse uses) the only way to change it is via colors option (with newer versions there is a simpler hideDate option).

We previously used require('debug').inspectOpts.colors = true to set the option, but that only works for the resolved version of debug and not all versions in the dependency tree.

A better fix is to use an environment variable to set the option for all debug versions. See https://github.com/visionmedia/debug/blob/80ef62a3af4df95250d77d64edfc3d0e1667e7e8/src/node.js#L120

@erezrokah erezrokah merged commit b2187ca into master Jul 13, 2020
@erezrokah erezrokah deleted the fix/lighthouse_logs branch July 13, 2020 12:02
erezrokah added a commit that referenced this pull request Jul 13, 2020
## [1.3.1](v1.3.0...v1.3.1) (2020-07-13)

### Bug Fixes

* **lighthouse:** hide logs timestamp when running in tty ([#50](#50)) ([b2187ca](b2187ca))
@erezrokah
Copy link
Contributor Author

🎉 This PR is included in version 1.3.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@@ -1,3 +1,6 @@
// prevent logger from prefixing a date when running in tty
process.env.DEBUG_COLORS = 'true';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that process.env.VAR = '...' sets the environment variable in the next plugins and/or build.command as well.
I am guessing this might not be intended. This could be fixed by restoring the previous value of this environment variable right after it's been used.
@erezrokah

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll change it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants