Skip to content

Commit

Permalink
Fix progress reporting during Playground load (#1915)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
brandonpayton authored Oct 16, 2024
1 parent bb2c69d commit b27c3c8
Showing 1 changed file with 28 additions and 20 deletions.
48 changes: 28 additions & 20 deletions packages/playground/remote/src/lib/offline-mode-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand All @@ -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;
}

Expand Down

0 comments on commit b27c3c8

Please sign in to comment.