Warn when an async matchCallback is used (#2591)

GoogleChrome · Oct 7, 2020 · ae29e17 · ae29e17
1 parent a6751b9
commit ae29e17
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 0 deletions.
diff --git a/packages/workbox-routing/src/Router.ts b/packages/workbox-routing/src/Router.ts
@@ -283,6 +283,17 @@ class Router {
       let params;
       const matchResult = route.match({url, sameOrigin, request, event});
       if (matchResult) {
+        if (process.env.NODE_ENV !== 'production') {
+          // Warn developers that using an async matchCallback is almost always
+          // not the right thing to do. 
+          if (matchResult instanceof Promise) {
+            logger.warn(`While routing ${getFriendlyURL(url)}, an async ` +
+                `matchCallback function was used. Please convert the ` +
+                `following route to use a synchronous matchCallback function:`,
+                route);
+          }
+        }
+
         // See https://github.com/GoogleChrome/workbox/issues/2079
         params = matchResult;
         if (Array.isArray(matchResult) && matchResult.length === 0) {

diff --git a/test/workbox-routing/sw/test-Router.mjs b/test/workbox-routing/sw/test-Router.mjs
@@ -8,6 +8,8 @@
 
 import {Route} from 'workbox-routing/Route.mjs';
 import {Router} from 'workbox-routing/Router.mjs';
+import {logger} from 'workbox-core/_private/logger.mjs';
+
 import {dispatchAndWaitUntilDone} from '../../../infra/testing/helpers/extendable-event-utils.mjs';
 import generateTestVariants from '../../../infra/testing/generate-variant-tests';
 
@@ -650,6 +652,27 @@ describe(`Router`, function() {
   });
 
   describe(`findMatchingRoute()`, function() {
+    it(`should log a warning in development when an async matchCallback is used`, function() {
+      if (process.env.NODE_ENV === 'production') return this.skip();
+
+      const loggerStub = sandbox.stub(logger, 'warn');
+
+      const router = new Router();
+      const route = new Route(async () => true, () => new Response());
+      router.registerRoute(route);
+
+      const url = new URL(location.href);
+      const request = new Request(url);
+      const event = new FetchEvent('fetch', {request});
+      const sameOrigin = true;
+
+      router.findMatchingRoute({url, sameOrigin, request, event});
+
+      expect(loggerStub.calledOnce).to.be.true;
+      // Just check for a snippet of the warning message.
+      expect(loggerStub.firstCall.args[0]).to.include('async');
+    });
+
     it(`should return the first matching route`, function() {
       const router = new Router();