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.

Sign up for GitHub

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

Jump to bottom

Simple PHP class structure #514

Closed
adamziel opened this issue Jun 3, 2023 · 2 comments
Closed

Simple PHP class structure #514

adamziel opened this issue Jun 3, 2023 · 2 comments
Labels
[Feature] PHP.wasm [Type] Developer Experience [Type] Discussion [Type] Exploration An exploration that may or may not result in mergable code

Comments

@adamziel
Copy link
Collaborator

adamziel commented Jun 3, 2023 •
edited
Loading

Playground has many classes and interfaces referring to similar concept of PHP. There’s:

  • IsomorphicLocalPHP - sync interface for running PHP in the current process
  • IsomorphicRemotePHP - same as above but async for running PHP remotely, e.g. in a worker
  • BasePHP - base implementation of IsomorphicLocalPHP
  • WebPHP - ditto, but with browser specific methods
  • NodePHP - ditto, but with node specific methods
  • UniversalPHP – an interface with common parts from all of the above that returns either synchronously or asynchronously
  • WebPHPEndpoint - Async implementation of IsomorphicRemotePHP that calls WebPHP running in a worker. Used by client apps
  • PlaygroundClient – A postMessage facade that calls WebPHPEndpoint from another frame
  • ...and probably more.

I saw developers get very confused about this structure, especially with regard to why UniversalPHP and PlaygroundClient are often interchangeable. It is difficult to understand the structure and easy to update one but forget about another (and miss it in a review, see #490).

Let’s discuss and see if there can be a simpler structure that allows for:

  • Isomorphic PHP interface that supports web, node, and other runtimes
  • Same-thread PHP execution with synchronous method
  • Remote PHP execution, e.g. in a worker, with asynchronous methods

If it cannot be simplified, then at least let’s find a way to raise alarm bells in TypeScript if the PR makes a change in BasePHP but not in the other ones.

cc @eliot-akira @dmsnell @wojtekn
@adamziel adamziel changed the title PHP: Simplify the inheritance structure PHP: Simplify the PHP interface structure Jun 3, 2023
@adamziel adamziel mentioned this issue Jun 3, 2023
Merged
@adamziel
Copy link
Collaborator Author

cc @reimic

@adamziel adamziel changed the title PHP: Simplify the PHP interface structure Simple PHP class structure Nov 22, 2023
This was referenced Nov 22, 2023
Closed
Open
This was referenced May 23, 2024
Closed
Merged
adamziel added a commit that referenced this issue May 25, 2024
@adamziel
Breaking: PHP: Remove NodePHP and WebPHP classes in favor of a single…
86da06b 
… "PHP" class (#1457)

> [!WARNING]  
> This is breaking change! Review the description below to assess the
impact on your app.

Consolidates the `BasePHP`, `NodePHP`, and `WebPHP` classes into a
single `PHP` class. This refactor reduces redundancy and clarifies the
PHP handling process across different environments.

## Examples

### NodePHP

**Before**

```ts
import { NodePHP } from '@php-wasm/node';
const php = await NodePHP.load('8.0');
```

**After**

```ts
import { loadNodeRuntime } from '@php-wasm/node';
import { PHP } from '@php-wasm/universal';
const php = new PHP(await loadNodeRuntime('8.0'));
```

### WebPHP

**Before**

```ts
import { WebPHP } from '@php-wasm/web';
const php = await WebPHP.loadRuntime('8.0');
```

**After**

```ts
import { loadWebRuntime } from '@php-wasm/web';
import { PHP } from '@php-wasm/universal';
const php = new PHP(await loadWebRuntime('8.0'));
```

### Mounting

**Before**

```ts
php.useHostFilesystem();
// or 
php.mount( '/home/users/adam/my-dir', '/my-dir-in-vfs');
```

**After**

```ts
import { NodeFSMount, useHostFilesystem } from '@php-wasm/node';
useHostFilesystem( php );
php.mount( new NodeFSMount( '/home/users/adam/my-dir' ), '/my-dir-in-vfs' );
```

Closes #1399

## Motivation

First, the public API surface of all the PHP classes and interfaces is
[overly
complex](#514).
This PR is a step towards simplifying it.

Second, in the [Boot
Protocol](#1398),
PR the `bootWordPress()` function requires separate `createPHP` and
`createPHPRuntime` callbacks just because Playground uses different
classes in node.js and in the browser. With this PR, `createPHP`
callback will no longer be needed anymore as `bootWordPress()` will just
always create a `new PHP()` instance.

## Public API Changes

- Removes `BasePHP`, `NodePHP`, and `WebPHP` classes in favor of a
single PHP class exported from `@php-wasm/universal`
- `php.useHostFilesystem()` was removed in favor of a decoupled function
`useHostFilesystem( php )` available in `@php-wasm/node`.
- `PHP.mount()` signature changed from `mount( hostPath: string,
vfsPath: string )` to `mount( mountable: Mountable, vfsPath: string )`
- Moves WebPHPEndpoint from `@php-wasm/web` to `@php-wasm/universal` and
renames it to `PHPWorker`. That class isn't specific to the web and
could be easily used by Node workers.
- Removes the `IsomorphicLocalPHP` interface. The PHP class is now the
source of truth for all derived worker, client, etc. implementations.
- Removes the `createPhpInstance()` option from `bootWordPress( options
)` in favor of always using the PHP class.
- Updates to Documentation: Adjusted examples and references in the
documentation to reflect the new `PHP` class.
- Refactoring of Test Suites: Updated tests to work with the new class
structure.

## Testing instructions

Confirm all the CI checks pass – this PR includes an extensive refactor
of all the tests.

Heads up @wojtekn @fluiddot @sejas
@adamziel
Copy link
Collaborator Author

adamziel commented Oct 24, 2024 •
edited
Loading

We're down to:

  • PHP – the same class used in the browser and in the CLI
  • UniversalPHP – an interface representing a local or remote PHP instance with limited set of methods
  • PlaygroundWorkerEndpoint - the worker API for interacting with Playground, exposes a boot() method to start the worker
  • PlaygroundClient – A JavaScript proxy object that forwards all the calls to PlaygroundWorkerEndpoint running in another frame
  • WebClientMixin – A remote.html-specific type that forwards PlaygroundClient calls to PlaygroundWorkerEndpoint and exposes DOM-specific methods such as goTo() that change the Playground iframe source

While it's 5 things and not 1 thing, I don't think there's a way to make this structure any simpler so I'll close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Feature] PHP.wasm [Type] Developer Experience [Type] Discussion [Type] Exploration An exploration that may or may not result in mergable code
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@adamziel