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

php-wasm/node : Invalid PHP runtime id #1579

Closed
mho22 opened this issue Jul 7, 2024 · 5 comments · Fixed by #1585
Closed

php-wasm/node : Invalid PHP runtime id #1579

mho22 opened this issue Jul 7, 2024 · 5 comments · Fixed by #1585
Assignees
@adamziel

Comments

@mho22
Copy link
Contributor

mho22 commented Jul 7, 2024 •
edited
Loading

While trying the new version of @php-wasm/node, I encountered errors linked with an invalid php runtime ID.

Here are the steps I followed :

  1. mkdir test
  2. cd test
  3. npm install @php-wasm/node @php-wasm/universal
  4. added "type": "module" in package.json`
  5. created test.js and added code :
import { PHP } from '@php-wasm/universal';
import { loadNodeRuntime } from '@php-wasm/node';

const php = new PHP( await loadNodeRuntime( '8.0' ) );

console.log( php );
  1. ran node test.js
file:///test/node_modules/@php-wasm/universal/index.js:1151
      throw new Error("Invalid PHP runtime id.");
            ^

Error: Invalid PHP runtime id.
    at PHP.initializeRuntime (file:///test/node_modules/@php-wasm/universal/index.js:1151:13)
    at new PHP (file:///test/node_modules/@php-wasm/universal/index.js:1060:167)
    at file:///test/test.js:4:13

Node.js v21.7.3
@adamziel adamziel self-assigned this Jul 8, 2024
@adamziel adamziel mentioned this issue Jul 8, 2024
Open
adamziel added a commit that referenced this issue Jul 8, 2024
@adamziel
Build: Treat all dependencies of php-wasm/node as external
0e8cb58 
Without this PR, the released `@php-wasm/node` package bundles other `@php-wasm`
packages, which caused multiple copies of, e.g. `@php-wasm/universal` to
be loaded and resulted in variable identity issues such as
#1579.

This PR marks all the dependent packages as external and ensures the
published package is lean and only contains its own code.

 ## Testing instructions

Run `nx build php-wasm-node` and confirm the resulting
`dist/packages/php-wasm/node/index.cjs` files does not contain any
external modules
@adamziel adamziel mentioned this issue Jul 8, 2024
Merged
adamziel added a commit that referenced this issue Jul 8, 2024
@adamziel
Build: Treat all dependencies of php-wasm/node as external (#1584)
e5d7347 
Without this PR, the released `@php-wasm/node` package bundles other
`@php-wasm`
packages, which caused multiple copies of, e.g. `@php-wasm/universal` to
be loaded and resulted in variable identity issues such as
#1579.

This PR marks all the dependent packages as external and ensures the
published package is lean and only contains its own code.

 ## Testing instructions

Run `nx build php-wasm-node` and confirm the resulting
`dist/packages/php-wasm/node/index.cjs` files does not contain any
external modules## Motivation for the change, related issues
@adamziel
Copy link
Collaborator

adamziel commented Jul 8, 2024 •
edited
Loading

Solved in release 0.9.12, thank you for reporting!
@adamziel adamziel closed this as completed Jul 8, 2024
@mho22
Copy link
Contributor Author

mho22 commented Jul 8, 2024

That was quick ! Thanks !
@adamziel
Copy link
Collaborator

adamziel commented Jul 8, 2024

Actually, it's only solved when "type": "module" is missing. I'll reopen.
@adamziel adamziel reopened this Jul 8, 2024
@mho22
Copy link
Contributor Author

mho22 commented Jul 8, 2024

You're right. I didn't work with "type": "module".
adamziel added a commit that referenced this issue Jul 8, 2024
@adamziel
php-wasm/node: Ship as ESM and CJS
b240066 
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 mentioned this issue Jul 8, 2024
Merged
@adamziel adamziel mentioned this issue Jul 8, 2024
Merged
@mho22 mho22 mentioned this issue Jul 8, 2024
Closed
@adamziel
Copy link
Collaborator

adamziel commented Jul 9, 2024 •
edited
Loading

This works as expected now in version 0.9.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@adamziel @mho22