Workbox: Workbox 3 module loading may depend on non-spec-compliant 'importScripts(...)' behavior (in some cases)

[josephliccini]

Library Affected:
workbox-sw 3.X

Browser & Platform:
Effect is really only present on MS Edge

Issue or Feature Request Description:
According to the SW Spec:

However, there are cases where workbox (via its Proxy) may run importScripts(...) during runtime.

Here's an example:

importScripts('/workbox-v3.2.0/workbox-sw.js');

workbox.precaching.precacheAndRoute([{ // <-- `workbox.precaching` implicitly calls `importScripts(...)` via ES6 Proxy.  As long as this happens during 'install', there's no issues
    file: '/app.js',
    revision: 'abc123'
}]);

self.addEventListener('fetch', async event => {
    const strategy = new workbox.strategies.CacheFirst({ /* ... */ }); // <-- This will call `importScripts(...)` via ES6 Proxy.  Since this happens outside of 'install', Edge (according to spec recommendations) will throw `NetworkError`
    const result = await strategy.handle({ event }) || Response.error();
    event.respondWith(Promise.resolve(result));
});

To be honest it's more of a Chrome / FF bug then a workbox bug, and maybe the spec should be updated and the browsers should align one way or another, _but_ for workbox to work correctly in Edge, a note in the docs or a descriptive console error on this case would be good.

You could probably try-catch and look for NetworkError, and if the worker state is not 'install', ask the user to importScripts(...) any workbox modules directly during install if they won't be imported during install by default.

So the workaround is:

importScripts('/workbox-v3.2.0/workbox-sw.js');
importScripts('/workbox-v3.2.0/workbox-precaching.prod.js'); // <-- hit during 'install'
importScripts('/workbox-v3.2.0/workbox-strategies.prod.js'); // <-- hit during 'install'

workbox.precaching.precacheAndRoute([{ // <-- `workbox.precaching` already available, no `importScripts(...)` needed
    file: '/app.js',
    revision: 'abc123'
}]);

self.addEventListener('fetch', async event => {
    const strategy = new workbox.strategies.CacheFirst({ /* ... */ }); // <-- `workbox.strategies` already available, no `importScripts(...)` needed
    const result = await strategy.handle({ event }) || Response.error();
    event.respondWith(Promise.resolve(result));
});

[jeffposnick]

Hmmm... so I know @gauntface looked into this when he implementing the lazy-loading-via-importScripts() in v3, and I think that what you describe is a limitation we were aware of. As I recall, we were under the impression that it was an issue for Chrome (and Firefox, etc.) as well as Edge. (Have you tested in Chrome to confirm that it actually works there?)

In your use case, I think the cleanest approach is just moving the stuff that triggers the lazy-loading outside of the fetch handler, like:

const strategy = new workbox.strategies.CacheFirst({ /* ... */ });

self.addEventListener('fetch', async event => {    
    const result = await strategy.handle({ event }) || Response.error();
    event.respondWith(Promise.resolve(result));
});

It's something we should do a better job of calling out in the docs, especially for folks who go with the roll-your-own-event-handler approach.

[josephliccini]

I have tested the above worker in Chrome (before workaround), and it does incorrectly succeed, where in Edge it produces NetworkError when it uses importScripts(...). Since the fetch handler throws an exception, the browser default behavior for this request is what happens on Edge, where in Chrome, it would do the workbox.strategies.CacheFirst behavior.

[jeffposnick]

Hmmm... weird!

In any case, does moving the lazy-loaded-bits outside of the fetch handler seem like a reasonable approach—one that's less heavyweight that having to explicitly call importScripts()?

[josephliccini]

I think for the general use case, yes that's a sufficient workaround (moving lazy loaded bits outside fetch handler).

Our case is a bit more complex 😄

Basically, we create cache names on the fly (user segregated caches) and to do that, we are calling new workbox.strategies.TheStrategy({ cacheName: createUserCacheName(userIdFromRequestHeader, ...) }) from within a fetch handler.

Placing importScripts(...) in the install phase is I think the only solution for now, but I don't think it's a mainstream requirement for most developers.

[jeffposnick]

(As an alternative, you can create your own custom builds of Workbox by consuming the ES2015 module source code, and import that single, static build into your top-level service worker.)

[josephliccini]

Great idea, I will have to give that a try!

We are already using webpack + TypeScript so it might make sense to just consume workbox as an ESM instead of importScripts(...)

[jeffposnick]

CC: @philipwalton who knows more about ESM usage, in case you run into any issues.

[josephliccini]

https://github.com/w3c/ServiceWorker/issues/1319

FYI looks like Chrome will align to Spec

[wanderview]

I'm aligning firefox to the spec as well.

[gauntface]

FYI I added a method to load modules up front because I was a little concerned this could crop up:

importScripts('.....workbox.js);

workbox.setConfig({
    modulePathCb: (moduleName) => {
        throw new Error(`Attempt was made to load module '${moduleName}', use loadModule() instead.`);
    }
});

workbox.loadModule('workbox-core');
workbox.loadModule('workbox-precaching');
workbox.loadModule('workbox-strategies');

Not as elegant as it could be - but it should work unless I'm missing something

[josephliccini]

@gauntface fantastic! that should work for our case. Feel free to close this issue since support for loading modules on demand outside of the Proxy usage is already supported.

[gauntface]

It might be worth discussing if we should be documenting this better and / or requiring this way or doing things.

Generally - I think most users of workbox won't hit this but I've stepped back from the project to up to @philipwalton and @jeffposnick for next steps.

[surajrautela]

So what i did was reduce my code to minimal and then test using the suggestions provided

// const variables
const workboxCdnUrl = "https://storage.googleapis.com/workbox-cdn/releases/3.2.0/workbox-sw.js";
const cacheName = "sp-cache-v1";
const expirationTimeInSeconds = 60 * 60 * 24;
const CDN_HOST_ENDS_WITH = '.akamaihd.net';

//main async function 
async function main(){
    var importWorkbox = (cdnUrl) => {
        importScripts(cdnUrl);
    }

 // importing workbox
 await  importWorkbox(workboxCdnUrl);

    // function to enable logging if mode value is debug it reads value from a cache and sets config //accordingly
    await (async function switchDebugMode()
    {
        var cache = await caches.open('sw-mode');;
        // var res=await cache.match("https://sw-mode/cache");
        // var json=JSON.parse(await res.text());
        //currently making it true for testing purpose
        if(1===1)
        {
        // if we want to enable logging for debugging puposes
            workbox.setConfig({
            debug: true
            });
        }
    })()
  // Loading Modules
    workbox.loadModule('workbox-core');
    workbox.loadModule('workbox-strategies');
    workbox.loadModule('workbox-routing');
    workbox.loadModule('workbox-cache-expiration');
    workbox.loadModule('workbox-cacheable-response');
    workbox.core.setLogLevel(workbox.core.LOG_LEVELS.log);

    // using stale while revalidate for all  urls 
    workbox.routing.registerRoute(
        new RegExp(".*"),
        workbox.strategies.staleWhileRevalidate({
            cacheName: cacheName
        })
    );
}

// calling the main function
main().then(console.log("service worker is installed"))

In chrome this works fine but in edge it agains throws an error

Unable to import module 'workbox-core' from 'https://storage.googleapis.com/workbox-cdn/releases/3.2.0/workbox-core.dev.js'.

Am i missing anything
@josephliccini @jeffposnick @gauntface

[jeffposnick]

All of your code is running inside of an async main(), which runs counter to the requirement that the imports happen synchronously during the initial execution of the service worker script.

There's really no benefit to using that async main() pattern, and as I pointed out on Stack Overflow, you're not going to be able to do something asynchronous to control whether Workbox is in debug mode or not.

My advice is to:

• Get rid of your async main() wrapper function.
• Find some way to synchronously control whether or not Workbox is in debug mode. As suggested on Stack Overflow, you could use a URL query parameter when you register the service worker.

[jeffposnick]

FYI, Chrome is moving forward with disallowing the non-compliant use of importScripts().

I'm going to close this issue and will submit a PR to update https://developers.google.com/web/tools/workbox/guides/get-started#importing_workbox to make it clearer that anything which might dynamically call importScripts() needs to happen either in the install handler or in the top-level script execution.