A boot() method to explicitly initialize the PHP worker

[adamziel]

Motivation for the change, related issues

Refactors the remote.html and worker-thread.ts boot flows.

• Before this PR, both files passed configuration using query arguments and relied on implicit logic surrounding the endpoint class
• After this PR, both API Endpoint classes expose a boot() method that is strongly typed, accepts the configuration values, and explicitly orchestrates all the boot logic when it is called.

This work may eventually enable:

• A reusable, isomorphic Worker implementation that can be reused in the browser and in a local CLI setup.
• Exposing bootWordPress() on a worker instance.
• Hot-swapping the PHP runtime and WordPress instance (e.g. by killing the current worker and starting a new one).
• Spawning new workers, not just PHP class instances, via PHPProcessManager.

Related to #1398

Follow-up work

• Explore spawning multiple workers for handling requests. This might also require:
  • Exposing the full boot protocol (bootWordPress()) on a worker instance instead of the simplified boot() method.
  • Load balancer across all workers handling the same site instance.
  • An OPFS Emscripten filesystem backend so that each worker can see the changes made by other workers. This is a potential blocker as Emscripten didn't seem to have a good way of doing that a few months ago.

Testing Instructions (or ideally a Blueprint)

• Confirm the tests are all green
• Carefully review the changes, point out anything that seems off

cc @brandonpayton @bgrgicak

[adamziel]

This is ready for review now.

Also, I wonder if bringing in files from git repositories could be expressed as a "read-only mount" 🤔 This would neatly interop with the CLI version.

[adamziel]

I know it's a large one and might be difficult to review. There's a few things I could extract, but unfortunately most of it is an atomic change. If nothing stands out, we could try deploying that on Monday with full readiness to rollback.

[brandonpayton]

I am not done reviewing but wanted to share progress so far because there is a blocker.

The Good:

• The name improvements in this PR help me so much. I started leaving comments about that, but there were so many that I eventually stopped commenting on it.
• Switching to explicit boot calls seems like a big improvement to clarity and function.

The Bad:

• This PR does not yet work in Safari. Trying to post a message containing FileSystemDirectoryHandle references is leading to a DataCloneError.
  

[bgrgicak]

That's a big PR 😅

+1 to what @brandonpayton mentioned. I like how the code is now easier to follow and better named.

This feels like a good, timing for a refactor, we added a lot of features to the boot flow recently and some changes were all over the place.

[brandonpayton]

I pushed 4d1a539 which passes around OPFS paths rather than FileSystemDirectoryHandles which due to the Safari structuredClone() not supporting that.

It looks like this may fix #1239 because Safari is loading Playground with browser storage locally after this change.

[brandonpayton]

There are some merge conflicts with trunk, so I'll look at those next.

[brandonpayton]

I missed some thing in the original switch from passing OPFS handles to passing OPFS paths, and those should be fixed now.

[brandonpayton]

I am currently seeing a bug with creating a new site on device storage:

index.tsx:139 Uncaught (in promise) DOMException: Unable to resolve path of OPFS directory handle.

Discovered while testing for this review comment:
https://github.com/WordPress/wordpress-playground/pull/1669/files#r1705279446

Looking into it.

[brandonpayton]

It looks like I was mistaken about the "fix" for this PR because we aren't just passing around opfs paths. FileSystemDirectoryHandles are also used for local FS access, so we cannot just pass paths around and assume they are for OPFS.

Since FileSystemDirectoryHandle instances cannot be transferred via postMessage() in Safari, we need a different solution.

Continuing to look into this.

[brandonpayton]

One thing to remember is that Safari doesn't support direct local FS access via showDirectoryPicker(). So we could consider passing FileSystemDirectoryHandles for the local FS case because local FS access doesn't work in Safari anyway.

[brandonpayton]

I worked on adjusting this to use the following types:

export type MountDevice =
	| {
		type: 'opfs';
		path: string;
	  }
	| {
		type: 'local-fs';
		// Since we have to prompt the user to get the FS handle,
		// we cannot simply save a path and recreate the handle
		// like we can do for OPFS. So we have to pass the handle between threads.
		handle: FileSystemDirectoryHandle;
	  };

export interface MountDescriptor {
	mountpoint: string;
	device: MountDevice;
}

Safari doesn't support structuredClone() for FileSystemHandle or the platform features we use to interact with the local device filesystem. Since we don't support local-fs sites on Safari, we shouldn't run into structuredClone() errors due to the FileSystemDirectoryHandles.

It also seemed good to rework how we were passing mount options, but there were issues due to progress callbacks making it into postMessage payloads and causing DataCloneErrors. So I'm working on backing those changes out so this can work.

[brandonpayton]

This PR should be working again in Safari and for local device sites.

@adamziel, please let me know if you have any thoughts on this. I'm planning to give this another read tomorrow or Monday and then merge if all is well.

[brandonpayton]

Confirmed this still works.

[brandonpayton]

I reviewed this again and found and fixed an issue with site reset.

[adamziel]

Fantastic work here @brandonpayton 🎉