From b27c3c84e409ef6a66dd599bb5a9c67eb4b464be Mon Sep 17 00:00:00 2001
From: Brandon Payton <brandon@happycode.net>
Date: Wed, 16 Oct 2024 20:31:11 +0100
Subject: [PATCH] Fix progress reporting during Playground load (#1915)

## Motivation for the change, related issues

Prior to this change, the service worker was not making the response
available to consumers until the response was placed in the offline
cache. It looks like the entire response had to be buffered before
consumers had a chance to observe response progress, and at that point,
there was no progress to observe because the download was complete.

Props to @adamziel for noticing this cause.

Fixes #1667

## Implementation details

This updates service worker request caching so that responses are made
available downstream before caching is complete.

## Testing Instructions (or ideally a Blueprint)

- CI
---
 .../remote/src/lib/offline-mode-cache.ts      | 48 +++++++++++--------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/packages/playground/remote/src/lib/offline-mode-cache.ts b/packages/playground/remote/src/lib/offline-mode-cache.ts
index 9aa4512fdb..ed5f837353 100644
--- a/packages/playground/remote/src/lib/offline-mode-cache.ts
+++ b/packages/playground/remote/src/lib/offline-mode-cache.ts
@@ -11,29 +11,34 @@ const promisedOfflineModeCache = caches.open(LATEST_CACHE_NAME);
 
 export async function cacheFirstFetch(request: Request): Promise<Response> {
 	const offlineModeCache = await promisedOfflineModeCache;
-	let response = await offlineModeCache.match(request, {
+	const cachedResponse = await offlineModeCache.match(request, {
 		ignoreSearch: true,
 	});
-	if (!response) {
+	if (cachedResponse) {
+		return cachedResponse;
+	}
+
+	/**
+	 * Ensure the response is not coming from HTTP cache.
+	 *
+	 * We never want to put a stale asset in CacheStorage as
+	 * that would break Playground.
+	 *
+	 * See service-worker.ts for more details.
+	 */
+	const response = await fetchFresh(request);
+	if (response.ok) {
 		/**
-		 * Ensure the response is not coming from HTTP cache.
-		 *
-		 * We never want to put a stale asset in CacheStorage as
-		 * that would break Playground.
-		 *
-		 * See service-worker.ts for more details.
+		 * Confirm the current service worker is still active
+		 * when the asset is fetched. Caching a stale request
+		 * from a stale worker has no benefits. It only takes
+		 * up space.
 		 */
-		response = await fetchFresh(request);
-		if (response.ok) {
-			/**
-			 * Confirm the current service worker is still active
-			 * when the asset is fetched. Caching a stale request
-			 * from a stale worker has no benefits. It only takes
-			 * up space.
-			 */
-			if (isCurrentServiceWorkerActive()) {
-				await offlineModeCache.put(request, response.clone());
-			}
+		if (isCurrentServiceWorkerActive()) {
+			// Intentionally do not await writing to the cache so the response
+			// promise can be returned immediately and observed for progress events.
+			// NOTE: This is a race condition for simultaneous requests for the same asset.
+			offlineModeCache.put(request, response.clone());
 		}
 	}
 
@@ -59,7 +64,10 @@ export async function networkFirstFetch(request: Request): Promise<Response> {
 	}
 
 	if (response.ok) {
-		await offlineModeCache.put(request, response.clone());
+		// Intentionally do not await writing to the cache so the response
+		// promise can be returned immediately and observed for progress events.
+		// NOTE: This is a race condition for simultaneous requests for the same asset.
+		offlineModeCache.put(request, response.clone());
 		return response;
 	}