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

Offline support #1483

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f315461
Add browser directory mounting support
bgrgicak Jun 2, 2024
c75c033
Merge branch 'trunk' into add/php-opfs-mount-trigger
bgrgicak Jun 2, 2024
8b964ac
Add basic caching TODO test limits and busting
bgrgicak Jun 2, 2024
12eb27c
Remove test code
bgrgicak Jun 2, 2024
3b2959a
Add note
bgrgicak Jun 2, 2024
6569a69
Merge branch 'trunk' into add/wp-version-caching
bgrgicak Jun 5, 2024
5593d9f
Remove cache from worker thread
bgrgicak Jun 5, 2024
b40da6c
Cache boot files
bgrgicak Jun 5, 2024
ddbe6e8
Update packages/php-wasm/web-service-worker/src/initialize-service-wo…
bgrgicak Jun 5, 2024
980592a
Add outdated cache cleanup
bgrgicak Jun 5, 2024
1702e0d
Clear only current version cache
bgrgicak Jun 5, 2024
027eeba
Merge branch 'trunk' into add/wp-version-caching
bgrgicak Jun 5, 2024
f03b3d4
More tests
bgrgicak Jun 5, 2024
4a15e6e
Fix linter
bgrgicak Jun 5, 2024
76fbbb0
Merge branch 'trunk' into add/wp-version-caching
bgrgicak Jun 11, 2024
e0c28ab
Merge branch 'trunk' into add/wp-version-caching
bgrgicak Jun 12, 2024
2a1ebaf
Merge branch 'trunk' into add/wp-version-caching
bgrgicak Jun 12, 2024
e34381b
Move cache to web-service-worker
bgrgicak Jun 12, 2024
b1182bf
Cache all unscoped urls
bgrgicak Jun 13, 2024
dfa9763
Add preloading TODO preload all files
bgrgicak Jun 13, 2024
dc1f71c
Load index.html when offline
bgrgicak Jun 14, 2024
c2fd2f9
Catch fetch error
bgrgicak Jun 18, 2024
c9ed3e4
Fix web-service-worker build
brandonpayton Jun 20, 2024
096c0bc
Merge branch 'trunk' into add/wp-version-caching
bgrgicak Jun 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/php-wasm/web-service-worker/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,10 @@
}
},
"build:bundle": {
"executor": "@nx/js:tsc",
"executor": "@nx/vite:build",
"outputs": ["{options.outputPath}"],
"options": {
"outputPath": "dist/packages/php-wasm/web-service-worker",
"main": "packages/php-wasm/web-service-worker/src/index.ts",
"tsConfig": "packages/php-wasm/web-service-worker/tsconfig.lib.json",
"assets": ["packages/php-wasm/web-service-worker/*.md"],
"updateBuildableProjectDepsInPackageJson": true
"outputPath": "dist/packages/php-wasm/web-service-worker"
}
},
"lint": {
Expand Down
69 changes: 69 additions & 0 deletions packages/php-wasm/web-service-worker/src/fetch-caching.ts
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'];
Copy link
Member

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?

Copy link
Collaborator Author

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.

Copy link
Member

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.


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',
Copy link
Member

Choose a reason for hiding this comment

The 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 /website-server/ directory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These files are required for Playground to load when offline.

These files don't appear to exist at all on the production site.

They aren't fetched by the service worker so they need to be preached in this way.

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
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this commented out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to cache the service worker script?

Copy link
Member

Choose a reason for hiding this comment

The 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.

Do we want to cache the service worker script?

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));
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are non-scoped requests typically requests for Playground web UI assets?

They request Playground web UI assets, WP remote assets, WP zip, PHP wasm, and JS.

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?

Yes, but I would prefer to get this out first and improve later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I would prefer to get this out first and improve later.

👍 works for me.

return;
}
const responsePromise = handleRequest(event);
if (responsePromise) {
event.respondWith(responsePromise);
const response = handleRequest(event);
if (response) {
event.respondWith(response);
Copy link
Member

Choose a reason for hiding this comment

The 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 handleRequest() become new fetch events that are handled above.

In my current mental model, fetches by the Service Worker do not trigger additional fetch events, but that might just be a poor assumption on my part.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.
There are also third-party remote requests like Google analytics, that we don't cache for now.

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.

Copy link
Member

@brandonpayton brandonpayton Jun 19, 2024

Choose a reason for hiding this comment

The 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:

  • scoped requests are not cached
  • scoped requests for remote WP assets are fetched by the Service Worker
  • fetch() by the Service Worker does not trigger subsequent fetch events (in my understanding and testing)
    ?

}
return;
});
}

Expand Down
14 changes: 14 additions & 0 deletions packages/php-wasm/web-service-worker/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ export default defineConfig({
}),
],

build: {
lib: {
// Could also be a dictionary or array of multiple entry points.
entry: 'src/index.ts',
name: 'php-wasm-web-service-worker',
fileName: 'index',
formats: ['es'],
},
rollupOptions: {
// External packages that should not be bundled into your library.
external: [],
},
},

test: {
globals: true,
cache: {
Expand Down
68 changes: 68 additions & 0 deletions packages/playground/remote/src/test/fetch-caching.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { areCacheKeysForSameFile, isValidFile } from '../lib/fetch-caching';

Check failure on line 1 in packages/playground/remote/src/test/fetch-caching.spec.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Cannot find module '../lib/fetch-caching' or its corresponding type declarations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is isValidFile() defined?


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', () => {
Copy link
Member

Choose a reason for hiding this comment

The 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);
});
});
2 changes: 1 addition & 1 deletion packages/playground/website/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"background_color": "#ffffff",
"display": "standalone",
"scope": "/",
"start_url": "/",
"start_url": "/website-server/",
"short_name": "WordPress Playground",
"description": "WordPress Playground",
"name": "WordPress Playground",
Expand Down
Loading