workbox icon indicating copy to clipboard operation
workbox copied to clipboard

Use a single fetch handler for all Router instances

Open divyeshsachan opened this issue 5 years ago • 7 comments
trafficstars

Library Affected: 5.0.0 workbox-google-analytics, workbox-routing, etc.

Browser & Platform: "all browsers"

Issue or Feature Request Description:

I'm using setDefaultHandler with offline-fallback page "sw-offline" to handle non-matching requests along with offline googleAnalytics.

However, the fetch routes added by googleAnalytics is not preferred over setDefaultHandler. For all "google analytics" requests, default route set by setDefaultHandler is used.

	// few registerRoute are used here

	workbox.googleAnalytics.initialize(); 
	const defaultNetworkOnlyHandler = async (args) => {
		console.log('Default:'+args.url);
		try {
			var defaultNetworkOnly = new NetworkOnly({
				fetchOptions: args.event.request.mode=='navigate'?{}:{credentials:'include'}
			});
			const response = await defaultNetworkOnly.handle(args);
			return response || await caches.match('sw-offline');
		} catch(error) {
			return await caches.match('sw-offline');
		}
	};
	setDefaultHandler(defaultNetworkOnlyHandler);

As it can be seen in console, "https://www.google-analytics.com/analytics.js" always uses default route of "NetworkOnly" instead of "NetworkFirst" route defined in workbox-google-analytics. Due to which analytics.js is not cached and throws an error in offline mode.

divyeshsachan avatar Feb 19 '20 09:02 divyeshsachan

Thanks for reporting this.

Moving your workbox.googleAnalytics.initialize() call so that it's earlier than any calls to registerRoute() should resolve the issue, but I understand that it's unexpected.

I'll keep this open to see if there's a clean way that we could handle this that isn't as dependent on ordering.

jeffposnick avatar Feb 25 '20 21:02 jeffposnick

Thanks for the response. The workaround suggested by you is working.

Moving your workbox.googleAnalytics.initialize() call so that it's earlier than any calls to registerRoute() should resolve the issue, but I understand that it's unexpected.

I have checked by moving, workbox.googleAnalytics.initialize() before all registerRoute() calls and its working as expected. i.e. "NetworkFirst" route defined in workbox-google-analytics is used instead of default route set by "setDefaultHandler".

I hope it will be fixed in upcoming version.

divyeshsachan avatar Feb 26 '20 09:02 divyeshsachan

We should probably do for workbox-google-analytics in v6 what we're doing for workbox-precaching, and have it use the default Router instance instead of creating its own.

CC: @philipwalton

jeffposnick avatar Apr 28 '20 15:04 jeffposnick

Actually, I forgot that workbox-precaching in v6 creates it own Router instead of using the default Router, so forget what I said about it being similar.

jeffposnick avatar Apr 28 '20 15:04 jeffposnick

Moving your workbox.googleAnalytics.initialize() call so that it's earlier than any calls to registerRoute() should resolve the issue, but I understand that it's unexpected.

@jeffposnick is workbox.googleAnalytics.initialize() supposed to be before any calls to registerRoute() anyway, or is it just for this specific use case?

nhoizey avatar Dec 01 '20 13:12 nhoizey

Primarily for this use case, but in general, putting it first is not a bad idea.

One of the things that might happen in v7 (which didn't make it into v6) is creating a single, global Router instance that all modules used. That would resolve this issue.

(CC: @philipwalton)

jeffposnick avatar Dec 01 '20 17:12 jeffposnick

@jeffposnick ok, thanks!

nhoizey avatar Dec 01 '20 18:12 nhoizey