Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vite] The offline path is requested regardless of the original path when online in dev mode #13987

Closed
vursen opened this issue Jun 16, 2022 · 9 comments · Fixed by #14151
Closed

Comments

@vursen
Copy link
Contributor

vursen commented Jun 16, 2022

Description of the bug

The service worker always requests the offline path in dev mode even if the original request path is different. Although this makes sense when the app is offline, it is absolutely incorrect when the app is online.

The related code fragment:

if (offlinePath === '.') {
registerRoute(
({ request, url }) => request.mode === 'navigate' && !isManifestEntryURL(url),
async ({ event }) => {
return networkFirst
.handle({ request: new Request(offlinePath), event })
.then(rewriteBaseHref);
}
)
}

Expected behavior

When the app is online, the service worker should not modify HTTP requests.

When the app is offline, the service worker should redirect HTTP requests to the offline path.

Minimal reproducible example

  1. Clone https://github.com/vaadin/cookbook
  2. Run mvn
  3. Open http://localhost:8080/css-from-java

Expected:

<meta name="description" content="This recipe shows how to inject small CSS class definitions into the current page from Java.">

Actual:

<meta name="description" content="Find ready to use solutions and code snippets for common development problems and UX patterns. Includes Vaadin examples in Java and TypeScript.">

Versions

  • Vaadin / Flow version: 23
  • Java version: 17
  • OS version: Mac OS
@vursen vursen added this to To do in Frontend build optimization via automation Jun 16, 2022
@vursen vursen changed the title [vite] The offline path is requested regardless of the actual path when online in dev mode [vite] The offline path is requested regardless of the original path when online in dev mode Jun 16, 2022
@mshabarov
Copy link
Contributor

Thanks for the issue @vursen !
What is the impact on users? Does this issue make the app not working or working incorrectly?

@mshabarov mshabarov added bug vite Tickets related to vite support labels Jun 16, 2022
@mshabarov mshabarov added this to Needs triage in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) via automation Jun 16, 2022
@mshabarov mshabarov moved this from Needs triage to New P2 in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Jun 16, 2022
@vursen
Copy link
Contributor Author

vursen commented Jun 16, 2022

What is the impact on users? Does this issue make the app not working or working incorrectly?

Not sure about the impact but it is presumably High: your app routes might not open correctly when you request them in the browser in dev mode and the service worker is enabled. I assume the routing would currently work correctly only in the case you have visited the index page first and then further routing is done on the client-side.

@Artur-
Copy link
Member

Artur- commented Jun 20, 2022

This issue makes it impossible to develop applications as it returns the wrong data in development mode in certain cases and you have no idea why

@vursen
Copy link
Contributor Author

vursen commented Jun 22, 2022

Background

There are two cases:

OFFLINE_PATH points to a custom offline path when it is specified by the user via the PWA annotation (e.g. offline.html).

In this case, Flow includes the offline path in the runtime manifest (sw-runtime-resources-precache.js) which causes the offline path to get pre-cached in both dev and prod modes. Flow also maintains the revision hash, updating it when the content of the offline fallback file is changed. In other words, no workarounds are needed, and this case works exactly the same as in Webpack.

OFFLINE_PATH points to the root path (= .).

This is the default case when no custom offline path is specified. In this case, index.html is considered the offline fallback and it is assumed that it will end up pre-cached as a result of being added to the build manifest (see injectManifestToSWPlugin). This works fine in prod mode where we have a build step. However, when using Vite, there is no build step in dev mode: everything is built on demand. This is why I had to come up with a workaround in sw.js for this case:

if (offlinePath === '.') {
registerRoute(
({ request, url }) => request.mode === 'navigate' && !isManifestEntryURL(url),
async ({ event }) => {
return networkFirst
.handle({ request: new Request(offlinePath), event })
.then(rewriteBaseHref);
}
)
}

For any navigation request (i.e when you open a resource via the address bar in the browser) that is not included in the manifest (build + runtime), the service worker returns the response as if the request was to the root path (= the default offline path). The idea was to get the default offline path cached as soon as any navigation request is made so that the application could be opened offline afterward.

This is not correct. The default offline path should be pre-cached (= index.html) in some other way and only returned when offline in order to allow navigation requests to their original resources when online.

Possible solution

The first solution that comes to mind is to configure Flow so that it would add the default offline path to the runtime manifest in dev mode when using Vite and then remove the workaround. This seems to be an optimal approach as it is pretty much in line with the way it works in prod mode. The only difference will be the manifest type. In production, the offline fallback path will come from the build manifest while in development mode, it will come from the runtime manifest.

References

Here is where the runtime manifest is created:

private String initializeRuntimeServiceWorker(
ServletContext servletContext) {
StringBuilder stringBuilder = new StringBuilder();
// List of files to precache
Collection<String> filesToCache = getIcons().stream()
.filter(PwaIcon::shouldBeCached).map(PwaIcon::getCacheFormat)
.collect(Collectors.toCollection(LinkedHashSet::new));
// When custom offlinePath is in use, it is also an offline resource to
// precache
if (pwaConfiguration.isOfflinePathEnabled()) {
filesToCache
.add(offlinePageCache(pwaConfiguration.getOfflinePath()));
}
// Offline stub to be shown within an <iframe> in the app shell
filesToCache
.add(offlinePageCache(PwaHandler.DEFAULT_OFFLINE_STUB_PATH));
// Add manifest to precache
filesToCache.add(manifestCache());
// Add user defined resources. Do not serve these via Webpack, as the
// file system location from which a resource is served depends on
// the (configurable) web app logic (#8996).
for (String resource : pwaConfiguration.getOfflineResources()) {
filesToCache.add(String.format(WORKBOX_CACHE_FORMAT,
resource.replaceAll("'", ""), servletContext.hashCode()));
}
// Precaching
stringBuilder.append("self.additionalManifestEntries = [\n");
stringBuilder.append(String.join(",\n", filesToCache));
stringBuilder.append("\n];\n");
return stringBuilder.toString();
}

Here is where the build manifest is created:

function injectManifestToSWPlugin(): PluginOption {
const rewriteManifestIndexHtmlUrl = (manifest) => {
const indexEntry = manifest.find((entry) => entry.url === 'index.html');
if (indexEntry) {
indexEntry.url = appShellUrl;
}
return { manifest, warnings: [] };
};
return {
name: 'vaadin:inject-manifest-to-sw',
enforce: 'post',
apply: 'build',
async closeBundle() {
await injectManifest({
swSrc: path.resolve(frontendBundleFolder, 'sw.js'),
swDest: path.resolve(frontendBundleFolder, 'sw.js'),
globDirectory: frontendBundleFolder,
globPatterns: ['**/*'],
globIgnores: ['**/*.br'],
injectionPoint: 'self.__WB_MANIFEST',
manifestTransforms: [rewriteManifestIndexHtmlUrl],
maximumFileSizeToCacheInBytes: 100 * 1024 * 1024 // 100mb,
});
}
};
}

@taefi
Copy link
Contributor

taefi commented Jul 5, 2022

Thanks, @vursen for the great explanation of the issue and suggestions for resolving it. However, I tried to reproduce it on my local as you described and I wasn't able to see Find ready to use solutions and code snippets for common development ... when navigating to http://localhost:8080/css-from-java, and it was always showing the expected This recipe shows how to inject small CSS class definitions into the current page from Java.. The following is what I've tried:

  1. Cloned the Cookbook
  2. Kept vaadin.version as it was (23.1.0)
  3. Ran mvn and also tried running the application directly from the main class
  4. First loaded http://localhost:8080 and then navigated to /css-from-java by entering the address in the address bar
  5. I also tried to navigate to /css-from-java by clicking on the first link.
  6. In another browser I navigated directly to /css-from-java without loading the http://localhost:8080/ first.
  7. Stopped the application, and tried refreshing the pages both on / and /css-from-java
  8. Started the application again, and navigated to the above paths

All the time it is loading the correct content. Tested on Firefox and Chrome, same results.
Would you please help me to find what am I missing while reproducing the issue?

@Artur-
Copy link
Member

Artur- commented Jul 5, 2022

You should see it when you have the server running, http://localhost:8080/css-from-java open and press refresh in the browser

@taefi
Copy link
Contributor

taefi commented Jul 5, 2022

Thanks! That's correct, I misunderstood it. It always adds the <meta> tag with the value for the root.

@taefi taefi self-assigned this Jul 5, 2022
taefi added a commit that referenced this issue Jul 7, 2022
taefi added a commit that referenced this issue Jul 7, 2022
taefi added a commit that referenced this issue Jul 7, 2022
taefi added a commit that referenced this issue Jul 8, 2022
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Jul 22, 2022
taefi added a commit that referenced this issue Jul 22, 2022
* fix: Fix ServiceWorker path requests in dev mode

Fixes: #13987

Co-authored-by: Marco Collovati <marco@vaadin.com>
Frontend build optimization automation moved this from To do to Done Jul 22, 2022
vaadin-bot pushed a commit that referenced this issue Jul 22, 2022
* fix: Fix ServiceWorker path requests in dev mode

Fixes: #13987

Co-authored-by: Marco Collovati <marco@vaadin.com>
mcollovati added a commit that referenced this issue Jul 22, 2022
Fixes: #13987

Co-authored-by: Soroosh Taefi <taefi.soroosh@gmail.com>
Co-authored-by: Marco Collovati <marco@vaadin.com>
taefi added a commit that referenced this issue Jul 22, 2022
* fix: Fix ServiceWorker path requests in dev mode

Fixes: #13987

Co-authored-by: Marco Collovati <marco@vaadin.com>

(cherry picked from commit 4e62c04)
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.1.6.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.2.0.beta1 and is also targeting the upcoming stable 23.2.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment