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

Remove NodePHP and WebPHP classes #1399

Closed
adamziel opened this issue May 15, 2024 · 0 comments · Fixed by #1457
Closed

Remove NodePHP and WebPHP classes #1399

adamziel opened this issue May 15, 2024 · 0 comments · Fixed by #1457

Comments

@adamziel
Copy link
Collaborator

adamziel commented May 15, 2024

The existence of separate NodePHP and WebPHP classes adds complexity without creating much value. The underlying runtime is the deciding factor when it comes to supported features anyway. Let's pull all their non-static methods into BasePHP, rename it to PHP, and use that. The static runtime-loading methods could be just regular functions. It should save us a lot of abstraction and typing conundrums.

Related to #514

adamziel added a commit that referenced this issue May 23, 2024
adamziel added a commit that referenced this issue May 25, 2024
… "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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant