-
Notifications
You must be signed in to change notification settings - Fork 273
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
Offline support #1483
Offline support #1483
Changes from all commits
f315461
c75c033
8b964ac
12eb27c
3b2959a
6569a69
5593d9f
b40da6c
ddbe6e8
980592a
1702e0d
027eeba
f03b3d4
4a15e6e
76fbbb0
e0c28ab
2a1ebaf
e34381b
b1182bf
dfa9763
dc1f71c
c2fd2f9
c9ed3e4
096c0bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import { logger } from '@php-wasm/logger'; | ||
|
||
const wpCacheKey = 'playground-cache'; | ||
|
||
const isCacheFull = async () => { | ||
if (navigator.storage && navigator.storage.estimate) { | ||
const estimatedStorage = await navigator.storage.estimate(); | ||
if (estimatedStorage.usage && estimatedStorage.quota) { | ||
return estimatedStorage.usage >= estimatedStorage.quota; | ||
} | ||
} | ||
return false; | ||
}; | ||
|
||
export const addCache = async (key: string, response: Response) => { | ||
if (await isCacheFull()) { | ||
logger.warn('Cache is full, not caching response'); | ||
return; | ||
} | ||
try { | ||
const clonedResponse = response.clone(); | ||
const cache = await caches.open(wpCacheKey); | ||
await cache.put(key, clonedResponse); | ||
} catch (e) { | ||
logger.warn('Failed to cache response', e); | ||
} | ||
}; | ||
|
||
export const getCache = async (key: string) => { | ||
const cache = caches.open(wpCacheKey); | ||
return await cache.then((c) => c.match(key)); | ||
}; | ||
|
||
const validHostnames = ['playground.wordpress.net', '127.0.0.1', 'localhost']; | ||
|
||
const isValidHostname = (url: URL) => { | ||
return validHostnames.includes(url.hostname); | ||
}; | ||
|
||
export const cachedFetch = async (request: Request): Promise<Response> => { | ||
const url = new URL(request.url); | ||
if (!isValidHostname(url)) { | ||
try { | ||
return await fetch(request); | ||
} catch (e) { | ||
logger.warn('Failed to fetch', request.url, e); | ||
return new Response('Failed to fetch', { status: 500 }); | ||
} | ||
} | ||
const cacheKey = url.pathname; | ||
const cache = await getCache(cacheKey); | ||
if (cache) { | ||
return cache; | ||
} | ||
const response = await fetch(request); | ||
await addCache(cacheKey, response); | ||
return response; | ||
}; | ||
|
||
const precachedResources: RequestInfo[] = [ | ||
'/website-server/', | ||
'/website-server/index.html', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we precaching these paths specifically? How do we adjust in production where there is no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These files are required for Playground to load when offline. They aren't fetched by the service worker so they need to be preached in this way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These files don't appear to exist at all on the production site.
Do you mean on initial load before the service worker can be registered? |
||
]; | ||
|
||
export const precacheResources = async (): Promise<any> => { | ||
return caches | ||
.open(wpCacheKey) | ||
.then((cache) => cache.addAll(precachedResources)); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ declare const self: ServiceWorkerGlobalScope; | |
|
||
import { awaitReply, getNextRequestId } from './messaging'; | ||
import { getURLScope, isURLScoped, setURLScope } from '@php-wasm/scopes'; | ||
import { cachedFetch, precacheResources } from './fetch-caching'; | ||
import { logger } from '@php-wasm/logger'; | ||
|
||
/** | ||
* Run this function in the service worker to install the required event | ||
|
@@ -13,6 +15,15 @@ import { getURLScope, isURLScoped, setURLScope } from '@php-wasm/scopes'; | |
export function initializeServiceWorker(config: ServiceWorkerConfiguration) { | ||
const { handleRequest = defaultRequestHandler } = config; | ||
|
||
self.addEventListener('install', (event) => { | ||
try { | ||
const precachePromise = precacheResources(); | ||
event.waitUntil(precachePromise); | ||
} catch (e) { | ||
logger.error('Failed to precache resources', e); | ||
} | ||
}); | ||
|
||
/** | ||
* The main method. It captures the requests and loop them back to the | ||
* Worker Thread using the Loopback request | ||
|
@@ -22,28 +33,28 @@ export function initializeServiceWorker(config: ServiceWorkerConfiguration) { | |
|
||
// Don't handle requests to the service worker script itself. | ||
if (url.pathname.startsWith(self.location.pathname)) { | ||
return; | ||
// return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this commented out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This return breaks caching. I'm still working on the PR so I didn't do anything with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to cache the service worker script? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To clarify the question: It looks like this only stops caching of the service worker script. Is that correct, and if so, why do we want to cache the service worker script? |
||
} | ||
|
||
// Only handle requests from scoped sites. | ||
// So – bale out if the request URL is not scoped and the | ||
// referrer URL is not scoped. | ||
if (!isURLScoped(url)) { | ||
let referrerUrl; | ||
try { | ||
referrerUrl = new URL(event.request.referrer); | ||
} catch (e) { | ||
return; | ||
} | ||
if (!isURLScoped(referrerUrl)) { | ||
// Let the browser handle uncoped requests as is. | ||
return; | ||
} | ||
if ( | ||
!isURLScoped(url) && | ||
!( | ||
event.request.referrer && | ||
isURLScoped(new URL(event.request.referrer)) | ||
) | ||
) { | ||
// Add caching for non-scoped requests | ||
event.respondWith(cachedFetch(event.request)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are non-scoped requests typically requests for Playground web UI assets? Would it be good for us to use a completely separate cache for app resources so we can easily flush the cache for those when we detect a new version of the Playground web UI? We could consider things like that more in a future iteration, but if we do that, I think it will be easy to forget our previous caching scheme(s) and leave cruft in the cache from earlier versions. Maybe that is OK... I'm not sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
They request Playground web UI assets, WP remote assets, WP zip, PHP wasm, and JS.
Yes, but I would prefer to get this out first and improve later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
👍 works for me. |
||
return; | ||
} | ||
const responsePromise = handleRequest(event); | ||
if (responsePromise) { | ||
event.respondWith(responsePromise); | ||
const response = handleRequest(event); | ||
if (response) { | ||
event.respondWith(response); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these scoped requests for things like WP assets? Does that mean we are not caching them? Or do the de-scoped requests made by In my current mental model, fetches by the Service Worker do not trigger additional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this helpes, but here is how requests work. All requests in the app go to this listener, no matter if it's a remote request or an internal Playground request. Remote requests should be cached to add offline support. These are files like the WP zip and remote static assets. Internal Playground requests are the same as scoped requests. This is where we reroute the request to go to PHP-wasm instead of going to a fetch. This would be a WordPress page load, an asset that exists inside the Playground filesystem like style.css, or a PHP script execution request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for explaining a bit. In my mental model yesterday, I'd forgotten that many WordPress files would be available locally. Note: The following question may be irrelevant if you are planning to proactively cache remote WP assets separately. For scoped requests for remote static assets that were eliminated from the WordPress build, won't those skip caching because:
|
||
} | ||
return; | ||
}); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import { areCacheKeysForSameFile, isValidFile } from '../lib/fetch-caching'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is |
||
|
||
describe('Valid files', () => { | ||
it('Should return true for valid files', () => { | ||
expect(isValidFile('/index.html')).toBe(false); | ||
expect(isValidFile('/')).toBe(false); | ||
|
||
expect(isValidFile('/assets/wp-6.4.zip')).toBe(true); | ||
expect(isValidFile('/assets/wp-6.4.1.zip')).toBe(true); | ||
expect(isValidFile('/assets/wp-6.4.1-cache_key.zip')).toBe(true); | ||
expect( | ||
isValidFile('https://playground.wordpress.net/assets/wp-6.4.zip') | ||
).toBe(true); | ||
|
||
expect(isValidFile('/assets/php_8_0.wasm')).toBe(true); | ||
expect(isValidFile('/assets/php_8_0_0.wasm')).toBe(true); | ||
expect(isValidFile('/assets/php_8_0_0-cache_key.wasm')).toBe(true); | ||
expect( | ||
isValidFile('https://playground.wordpress.net/assets/php_8_0.wasm') | ||
).toBe(true); | ||
|
||
expect(isValidFile('/assets/sqlite-database-integration.zip')).toBe( | ||
true | ||
); | ||
expect( | ||
isValidFile('/assets/sqlite-database-integration-cache_key.zip') | ||
).toBe(true); | ||
expect( | ||
isValidFile( | ||
'https://playground.wordpress.net/assets/sqlite-database-integration.zip' | ||
) | ||
).toBe(true); | ||
}); | ||
|
||
it('Shoud return true if the cache keys are for the same file', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: "Shoud" |
||
expect( | ||
areCacheKeysForSameFile( | ||
'/assets/wp-6.4.zip', | ||
'/assets/wp-6.4-cache_key.zip' | ||
) | ||
).toBe(true); | ||
expect( | ||
areCacheKeysForSameFile( | ||
'/assets/php_8_0-old_key.wasm', | ||
'/assets/php_8_0-cache_key.wasm' | ||
) | ||
).toBe(true); | ||
expect( | ||
areCacheKeysForSameFile( | ||
'/assets/sqlite-database-integration.zip', | ||
'/assets/sqlite-database-integration-cache_key.zip' | ||
) | ||
).toBe(true); | ||
|
||
expect( | ||
areCacheKeysForSameFile( | ||
'/assets/wp-6.4.zip', | ||
'/assets/wp-6.4.1-cache_key.zip' | ||
) | ||
).toBe(false); | ||
expect( | ||
areCacheKeysForSameFile( | ||
'/assets/wp-6.4.zip', | ||
'/assets/php_8_0.wasm' | ||
) | ||
).toBe(false); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to validate hostnames? What if people want to host their own Playground?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't but I didn't want to cache their party scripts like Google Analytics.
But we could just cache everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, I had no idea Service Workers also receive fetch events for requests to third party resources. But it appears to be true.
It looks like
fetchEvent.currentTarget.registration.scope
is available, so we could filter based on that rather than a specific set of hostnames.fetchEvent.currentTarget
points to the ServiceWorkerGlobalScope. ServiceWorkerGlobalScope exposes the ServiceWorkerRegistration as its registration property, and ServiceWorkerRegistration has the scope property.