Skip to content
New issue

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

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

Already on GitHub? # to your account

Workbox debug messages clutter console #170

Closed
iandunn opened this issue Jun 11, 2019 · 9 comments · Fixed by #173
Closed

Workbox debug messages clutter console #170

iandunn opened this issue Jun 11, 2019 · 9 comments · Fixed by #173
Milestone

Comments

@iandunn
Copy link
Collaborator

iandunn commented Jun 11, 2019

Problem

Workbox's debug mode is currently tied to WP_DEBUG:

https://github.com/xwp/pwa-wp/blob/a4bcf446cda9f6e308da356eb665668bb9c5831f/wp-includes/components/class-wp-service-worker-configuration-component.php#L69-L70

The messages from Workbox are so noisy that they clutter the log to the point where it's very distracting, and almost unusable for me. That's still the case with the 700+ console.debug messages filtered off, so that only console.log messages are being displayed.

I think the devex here should conform to the same principle that the UX would: under normal circumstances, nothing should be output to the console. For developers, having WP_DEBUG enabled is a normal circumstance. I should only see Workbox logging when I explicitly enable it, in order to debug something.

Potential Solution

Ideally, I don't think this should be tied to WP_DEBUG at all, but should have a separate constant like WORKBOX_DEBUG.

At the very least, though, there should be a filter around $options so that developers can disable that for themselves.

I haven't thought a lot about the proper way to solve this, though, so I'm not strongly tied to either of those, they were just the first ideas that came to mind.

@westonruter
Copy link
Collaborator

You're right. Tying this to WP_DEBUG is not ideal.

There is only really one config option, debug, we'd want to be allow filterable: https://developers.google.com/web/tools/workbox/reference-docs/latest/workbox#.setConfig

So either we add a filter specifically for debug, or we add a constant for debug.

Alternatively, what about using the SCRIPT_DEBUG constant? This would be more appropriate than WP_DEBUG actually. This should also be used in:

https://github.com/xwp/pwa-wp/blob/a4bcf446cda9f6e308da356eb665668bb9c5831f/wp-includes/components/class-wp-service-worker-configuration-component.php#L57-L62

This is particularly appropriate here because here it is forcing the use of non-production Workbox files, which is what the constant does in core as well.

@iandunn
Copy link
Collaborator Author

iandunn commented Jun 11, 2019

🤔

I typically leave SCRIPT_DEBUG enabled all of the time as well, but I've done that for so long that I don't actually remember why :)

I can't think of any tangible consequences to leaving it off most of the time, so tying Workbox's debug to that seems acceptable to me.

Although, if I were trying to debug something w/ the bundled version of React or whatnot, I probably still wouldn't want Workbox stuff cluttering the console, so there does seem to be a fundamental difference between "i want source files instead of compiled" and "i want workbox debug messages", even if there's sometimes overlap in the context of working on PWAs.

So, I think a filter/constant specifically for Workbox debugging would probably be ideal IMO, but using SCRIPT_DEBUG wouldn't bother me all that often either.

@westonruter
Copy link
Collaborator

In regards to the log levels, I just saw this in Migrate from Workbox v3 to v4:

In Workbox v3 the verbosity of log levels could be controlled via the workbox.core.setLogLevel() method, which you'd pass one of the values in the workbox.core.LOG_LEVELS enum. You could also read the current log level via workbox.core.logLevel.

In Workbox v4, all of these have been removed since all modern developer tools now ship with rich log filtering capabilities (see filtering the console output for Chrome Dev Tools).

@westonruter
Copy link
Collaborator

I just remembered why it is important to have SCRIPT_DEBUG enabled, when you're running trunk:

image

@westonruter
Copy link
Collaborator

Otherwise, with the changes in #173 I can indeed set SCRIPT_DEBUG to false and I then see no log messages.

@westonruter
Copy link
Collaborator

@felixarntz What do you think of adding a new SERVICE_WORKER_DEBUG constant?

@westonruter
Copy link
Collaborator

Related: GoogleChrome/workbox#2284

@westonruter
Copy link
Collaborator

We should be able to now to define __WB_DISABLE_DEV_LOGS to turn off these messages in dev mode. See GoogleChrome/workbox#2284

@westonruter
Copy link
Collaborator

Now that Workbox v5 is almost ready for release, there is now a mechanism available to disable the logging. See #173 for the proposed constant to opt-in to the logging which is only enabled now if SCRIPT_DEBUG is enabled.

@westonruter westonruter added this to the 0.4 milestone Mar 31, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants