Move URL rewriting from the service worker to the request handler

[adamziel]

Requests resolved directly via the JS API (playground.request()) return error 404 if URL rewriting is required to resolve them. For example, playground.request({ url: '/glotpress/projects }); wouldn't get resolved as /index.php/glotpress/projects. This is because Playground simulates the required .htaccess rules in the service worker:

wordpress-playground/packages/playground/remote/service-worker.ts

Lines 51 to 64 in 454fec3

	if (workerResponse.status === 404) {
	for (const url of rewriteWordPressUrl(unscopedUrl, scope!)) {
	rewrittenUrlString = url.toString();
	workerResponse = await convertFetchEventToPHPRequest(
	await cloneFetchEvent(event, rewrittenUrlString)
	);
	if (
	workerResponse.status !== 404 ||
	workerResponse.headers.get('x-file-type') === 'static'
	) {
	break;
	}
	}
	}

And only fetch() requests are affected. Instead, Playground should apply the following .htaccess rules in the request handler:

RewriteEngine On
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteBase /
RewriteRule ^index\.php$ - [L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /index.php [L]

Since PHPRequestHandler is generic and specific to @php-wasm, Playground should provide its own WordPress-specific handler, perhaps via the @wp-playground/wordpress package?

cc @akirk @bgrgicak

[bgrgicak]

Makes sense to me. We should rewrite all requests the same way htaccess does it.
@adamziel feel free to assign this one to me.

[bgrgicak]

Since PHPRequestHandler is generic and specific to @php-wasm, Playground should provide its own WordPress-specific handler, perhaps via the @wp-playground/wordpress package?

I wouldn't replace PHPRequestHandler. Adding rewrites similarly to what we have today should be enough.

[adamziel]

Adding rewrites similarly to what we have today should be enough.

@bgrgicak I wouldn't add WordPress-specific logic to any @php-wasm package, but perhaps the generic PHPRequestHandler could accept an option like rewrite: (url: string, php: BasePHP) => string and then the WordPress package would just ship a WordPress-specific rewrite handler.

[adamziel]

Surfacing this comment from a related PR:

My follow-up question would be: do we still need this code?

wordpress-playground/packages/playground/remote/service-worker.ts

Lines 51 to 64 in 454fec3

	if (workerResponse.status === 404) {
	for (const url of rewriteWordPressUrl(unscopedUrl, scope!)) {
	rewrittenUrlString = url.toString();
	workerResponse = await convertFetchEventToPHPRequest(
	await cloneFetchEvent(event, rewrittenUrlString)
	);
	if (
	workerResponse.status !== 404 ||
	workerResponse.headers.get('x-file-type') === 'static'
	) {
	break;
	}
	}
	}

If yes, then #1054 is still relevant and we'll still need to support that logic at the request handler level, otherwise the service worker will rewrite multisite URLs correctly, but other PHP.wasm consumers like WP-NOW and VS Code won't.

If not, then #1054 is indeed solved.

[bgrgicak]

@adamziel this code isn't needed. I removed it in this PR #1083