From b4a8c3e683f4aa309f8f409c9ee99be7cb032525 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kari=20So=CC=88derholm?= <haprog@vaadin.com>
Date: Wed, 4 Nov 2020 17:29:09 +0200
Subject: [PATCH] fix(offline): navigation fallback (#9280)

Fixes #9218.

This makes navigation fallback (offline start) use the same service worker logic as we use for `offlinePath`, just using the app shell route `/` instead of the route to offline page for the cache entry to be returned. I did some manual testing and this does seem to fix the issues described in #9218.

The added IT seems to be a false negative (always passes) and doesn't fail even without the fix, test can be fixed in #9314
---
 flow-server/src/main/resources/sw.ts          | 52 ++++++++-----------
 .../vaadin/flow/navigate/ServiceWorkerIT.java | 32 ++++++++++++
 2 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/flow-server/src/main/resources/sw.ts b/flow-server/src/main/resources/sw.ts
index b76cfae7394..06e527ced7d 100644
--- a/flow-server/src/main/resources/sw.ts
+++ b/flow-server/src/main/resources/sw.ts
@@ -5,43 +5,33 @@ import {skipWaiting, clientsClaim} from 'workbox-core';
 import {matchPrecache, precacheAndRoute} from 'workbox-precaching';
 import {NavigationRoute, registerRoute} from 'workbox-routing';
 import {PrecacheEntry} from 'workbox-precaching/_types';
-import {NetworkFirst, NetworkOnly} from 'workbox-strategies';
+import {NetworkOnly} from 'workbox-strategies';
 
 skipWaiting();
 clientsClaim();
 
-let navigationFallback;
-// @ts-ignore: OFFLINE_PATH_ENABLED is defined by webpack.generated.js
-if (OFFLINE_PATH_ENABLED) {
-  // @ts-ignore
-  const offlinePath = '/' + OFFLINE_PATH;
-  const networkOnly = new NetworkOnly();
-  navigationFallback = new NavigationRoute(async (params: any) => {
-    // Use offlinePath fallback if offline was detected
-    if (!self.navigator.onLine) {
-      const offlinePathPrecachedResponse = await matchPrecache(offlinePath);
-      if (offlinePathPrecachedResponse) {
-        return offlinePathPrecachedResponse;
-      }
+const appShellPath = '/';
+// @ts-ignore: OFFLINE_PATH_ENABLED and OFFLINE_PATH are defined by webpack.generated.js
+const offlinePath = OFFLINE_PATH_ENABLED ? '/' + OFFLINE_PATH : appShellPath;
+const networkOnly = new NetworkOnly();
+const navigationFallback = new NavigationRoute(async (params: any) => {
+  // Use offlinePath fallback if offline was detected
+  if (!self.navigator.onLine) {
+    const offlinePathPrecachedResponse = await matchPrecache(offlinePath);
+    if (offlinePathPrecachedResponse) {
+      return offlinePathPrecachedResponse;
     }
+  }
+
+  // Sometimes navigator.onLine is not reliable, use fallback to offlinePath
+  // also in case of network failure
+  try {
+    return await networkOnly.handle(params);
+  } catch (error) {
+    return (await matchPrecache(offlinePath)) || error;
+  }
+});
 
-    // Sometimes navigator.onLine is not reliable, use fallback to offlinePath
-    // also in case of network failure
-    try {
-      return await networkOnly.handle(params);
-    } catch (error) {
-      return (await matchPrecache(offlinePath)) || error;
-    }
-  });
-} else {
-  const appShellCacheKey = '/';
-  navigationFallback = new NavigationRoute(new NetworkFirst({
-    plugins: [
-      // Always use app shell cache key instead of original request URL
-      { cacheKeyWillBeUsed: async () => appShellCacheKey },
-    ]
-  }));
-}
 registerRoute(navigationFallback);
 
 // @ts-ignore: __WB_MANIFEST is injected by the InjectManifest plugin
diff --git a/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/ServiceWorkerIT.java b/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/ServiceWorkerIT.java
index b11f34ccc1d..7db1a249477 100644
--- a/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/ServiceWorkerIT.java
+++ b/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/ServiceWorkerIT.java
@@ -68,6 +68,38 @@ public void testOfflineStart() throws IOException {
         }
     }
 
+    @Test
+    public void testOfflineStart_nonRootReload() throws IOException {
+        getDriver().get(getRootURL() + "/");
+        waitForServiceWorkerReady();
+
+        // Set offline network conditions in ChromeDriver
+        setConnectionType(NetworkConnection.ConnectionType.AIRPLANE_MODE);
+
+        try {
+            $("main-view").first().$("a").id("menu-another").click();
+
+            // Wait for component inside shadow root as there is no vaadin
+            // to wait for as with server-side
+            waitUntil(input -> $("another-view").first().$("div").id("another-content")
+                    .isDisplayed());
+
+            // Reload the page in offline mode
+            executeScript("window.location.reload();");
+            waitUntil(webDriver -> ((JavascriptExecutor) driver)
+                    .executeScript("return document.readyState")
+                    .equals("complete"));
+
+            MatcherAssert.assertThat(getDriver().getCurrentUrl(),
+                    CoreMatchers.endsWith("/another"));
+            Assert.assertTrue(getInShadowRoot(findElement(By.tagName("another-view")),
+                    By.id("another-content")).isDisplayed());
+        } finally {
+            // Reset network conditions back
+            setConnectionType(NetworkConnection.ConnectionType.ALL);
+        }
+    }
+
     @Test
     public void openTsView_goOffline_navigateToOtherTsView_navigationSuccessful() throws IOException {
         getDriver().get(getRootURL() + "/about");