window.__WB_DISABLE_DEV_LOGS declaration

[jeffposnick]

R: @philipwalton

We need this extra declaration because workbox-core's logger can be used in the window context as well.

[westonruter]

What is the proper way to use this __WB_DISABLE_DEV_LOGS?

I've tried adding self.__WB_DISABLE_DEV_LOGS = true; to the service worker JS, for example (note this is PHP-generated JS):

https://gist.github.com/westonruter/f6e4dda95a881bfa1a98e305673971b2#file-service-worker-with-__wb_disable_dev_logs-js-L10

I also added window.__WB_DISABLE_DEV_LOGS = true; to the host page.

But to no avail, I'm still seeing a lot of logging in the console:

Have I misunderstood what this is intending to do?

[philipwalton]

Hmmm, that looks correct to me. Can you confirm that the code you're running is importing workbox v5.0.0-rc.1? That's the release this change landed in.

[westonruter]

@philipwalton Yes, I can confirm it is importing v5.0.0-rc.1.

I've deployed the service worker code to https://dev-westonruter.pantheonsite.io/

Service worker is located at https://dev-westonruter.pantheonsite.io/?wp_service_worker=1

It includes self.__WB_DISABLE_DEV_LOGS = true. I even tried moving it before the importScripts() call:

self.__WB_DISABLE_DEV_LOGS = true;
importScripts( "https://dev-westonruter.pantheonsite.io/wp-content/plugins/pwa/wp-includes/js/workbox/workbox-sw.js" );

But that didn't seem to make a difference.

You can see from workbox-sw.js that the version is 5.0.0-rc.1.

Aside: Why is this dev log disabling not passed via some configuration option via workbox.setConfig() as opposed to a global variable?

[jeffposnick]

My guess would be that we need to change the unconditional

workbox/packages/workbox-core/src/_private/logger.ts

Line 24 in 6845f53

self.__WB_DISABLE_DEV_LOGS = false;

to make it conditional, like

if (!'__WB_DISABLE_DEV_LOGS' in self) {
  self.__WB_DISABLE_DEV_LOGS = false;
}

to ensure that folks who explicitly set self.__WB_DISABLE_DEV_LOGS ahead of time don't have that value clobbered.

To answer your second question, we're expecting that with Workbox v5, folks will start moving away from using the pre-built bundles loaded via workbox-sw (as you're using) and instead start creating their own custom bundles by consuming the ES modules. That makes it awkward to hang this toggle off of something like workbox.core.setConfig() since we can't count on folks actually including the setConfig() method in their bundled service worker code.

That was the thinking, at least! If you feel strongly about the global approach, we're open to feedback.

[philipwalton]

Aside: Why is this dev log disabling not passed via some configuration option via workbox.setConfig() as opposed to a global variable?

The other problem with adding a method to the public API to disable logging is it makes it very difficult (if not impossible) to ensure all the logger code doesn't make it into the production build.

Right now we wrap all internal occurrences of the logger methods in a conditional, but if we had some sort of public workbox.core.setConfig() method (as we used to), then we couldn't control how developers used it. If they failed to put it in a conditional and then generated their own SW bundles, there'd be a high likelihood that all the logger code would make it into their production bundled (even though it wasn't being used).

[westonruter]

Thanks a lot for the responses. This makes sense to me.

we're expecting that with Workbox v5, folks will start moving away from using the pre-built bundles loaded via workbox-sw (as you're using) and instead start creating their own custom bundles by consuming the ES modules.

I do hope there is no plan to deprecate/remove workbox-sw. Being able to lazily load the modules as needed is critical in the WordPress context where we don't know up-front which modules will be used when new themes/plugins request them later.