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: Load correct php.wasm paths in the built Node.js packages #1877

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Oct 9, 2024

Gets @php-wasm/node and all the other CLI packages to load the .wasm
binaries from the correct path after #1867 introduced JSPI and #1873 made
it pass all the tests in Node.js

Technical details

In the repo, php.js files are stored in php_wasm/node/jspi or php_wasm/node/asyncify.
They start with a line like this:

const dependencyFilename = __dirname + '/8_0_30/php_8_0.wasm';

After the build, the contents are concatenated into a single file, which
breaks the dependencyFilename variable. This plugin corrects that by
replacing __dirname with the correct value such as 'jspi' or 'asyncify'.

The implementation is naive and assumes the substrings __dirname is only used
as a variable, are not a part of any other name, and is not seen in any string
literals.

Test plan

Build the production packages with nx build php-wasm-node and run the PHP CLI package to confirm it doesn't crash. I couldn't get npm link to work so I created a dist/packages/php-wasm/node_modules/@php-wasm directory and linked all the built packages there with ln -s.

 Gets `@php-wasm/node` and all the other CLI packages to load the .wasm
 binaries from the correct path after #1867 introduced JSPI and #1873 made
 it pass all the tests in Node.js

 ## Technical details

In the repo, php.js files are stored in php_wasm/node/jspi or php_wasm/node/asyncify.
 They start with a line like this:

 const dependencyFilename = __dirname + '/8_0_30/php_8_0.wasm';

 After the build, the contents are concatenated into a single file, which
 breaks the dependencyFilename variable. This plugin corrects that by
 replacing __dirname with the correct value such as 'jspi' or 'asyncify'.

 The implementation is naive and assumes the substrings __dirname is only used
 as a variable, are not a part of any other name, and is not seen in any string
 literals.

 ## Test plan

Build the production packages with `nx build php-wasm-node` and run the
PHP CLI package to confirm it doesn't crash. I couldn't get npm link to
work so I created a `dist/packages/php-wasm/node_modules/@php-wasm`
directory and linked all the built packages there with `ln -s`.
@adamziel adamziel merged commit 620f450 into trunk Oct 9, 2024
9 checks passed
@adamziel adamziel deleted the jspi-build-php-wasm-node branch October 9, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant