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

php-wasm/node: Ship as ESM and CJS #1585

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jul 8, 2024

The @php-wasm/node package used to be CommonJS only. This created tricky dependency identity problems in ESM applications, e.g. in #1577 the main script imports the ESM version of @php-wasm/universal, but @php-wasm/node imports the CJS version of it.

This PR ensures @php-wasm/node has an ESM version available.

This is first step towards solving #1583

Closes #1579 Closes #1577

Testing instructions

Run npm run build, copy the built packages from dist/packages/php-wasm to node_modules in another directory, create a test.js script with the following contents:

import { PHP } from '@php-wasm/universal';
import { loadNodeRuntime, useHostFilesystem } from '@php-wasm/node';

const php = new PHP(await loadNodeRuntime('8.0'));
console.log("Hey");
console.log(await php.run({ code: 'echo "Hello, World!";' }));

Then create a package.json file with {"type":"module"} and confirm that running test.js works. Then create test.cjs file as follows:

const { PHP } = require('@php-wasm/universal');
const { loadNodeRuntime } = require('@php-wasm/node');

async function main() {
    const runtimeId = await loadNodeRuntime('8.0');
    console.log({runtimeId})
    const php = new PHP(runtimeId);

    console.log(php);
}
main();

And confirm this one works, too.

The `@php-wasm/node` package used to be CommonJS only. This created
tricky dependency identity problems in ESM applications, e.g. in #1577
the main script imports the ESM version of `@php-wasm/universal`, but
`@php-wasm/node` imports the CJS version of it.

This PR ensures `@php-wasm/node` has an ESM version available.

This is first step towards solving #1583

Closes #1579
Closes #1577

 ## Testing instructions

Run `npm run build`, copy the built packages from `dist/packages/php-wasm` to
node_modules in another directory, create a test.js script with the
following contents:

```
import { PHP } from '@php-wasm/universal';
import { loadNodeRuntime, useHostFilesystem } from '@php-wasm/node';

const php = new PHP(await loadNodeRuntime('8.0'));
console.log("Hey");
console.log(await php.run({ code: 'echo "Hello, World!";' }));
```

Then create a package.json file with `{"type":"module"}` and confirm
that running test.js works. Then create test.cjs file as follows:

```js
const { PHP } = require('@php-wasm/universal');
const { loadNodeRuntime } = require('@php-wasm/node');

async function main() {
    const runtimeId = await loadNodeRuntime('8.0');
    console.log({runtimeId})
    const php = new PHP(runtimeId);

    console.log(php);
}
main();
```

And confirm this one works, too.
@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Priority] High [Package][@php-wasm] Node labels Jul 8, 2024
@adamziel adamziel merged commit 8aed120 into trunk Jul 8, 2024
1 check passed
@adamziel adamziel deleted the ship-php-wasm-node-as-esm branch July 8, 2024 13:52
adamziel added a commit that referenced this pull request Jul 9, 2024
## Motivation for the change, related issues

Related to #1587

The recent changes to [publish `@php-wasm/node` as
ESM](#1585) broke
the `.wasm` files paths in the published package. Specifically,
`__dirname + '/php_8_0.wasm'` became
`/home/runner/work/wordpress-playground/wordpress-playground/node_modules/@php-wasm/node/php_8_0.wasm`
which is undesirable.

This PR polyfills the `__dirname` variable with ESM-compliant
alternatives in the ESM version of the `@php-wasm/node` package.

## Testing instructions

* Run `npm run build`
* Confirm the built `@php-wasm/node` package has the correct `.wasm`
files paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package][@php-wasm] Node [Priority] High [Type] Bug An existing feature does not function as intended
1 participant