Use raw PHP-WASM to run WP-CLI commands#2376
Use raw PHP-WASM to run WP-CLI commands#2376fredrikekelund merged 8 commits intodev/studio-cli-i2from
Conversation
|
Based on a quick test, it works as expected. It's faster than 5 seconds, but still much slower than the native I noticed that table formatting is different. Here is the native And one from Studio CLI: |
| ...args, | ||
| ] ); | ||
| // Setup SSL certificates | ||
| php.writeFile( '/tmp/ca-bundle.crt', rootCertificates.join( '\n' ) ); |
There was a problem hiding this comment.
Why do we need to set up an SSL certificate?
There was a problem hiding this comment.
I haven't tested without it, but WP-CLI sends network requests as part of several different commands, so I kind of assume this is needed
There was a problem hiding this comment.
I have tested running several commands, e.g. wp plugin install with the code commented out and they run without issues.
| createNodeFsMountHandler( loaderMuPluginHostPath ) | ||
| ); | ||
| await php.mount( '/tmp/wp-cli.phar', createNodeFsMountHandler( getWpCliPharPath() ) ); | ||
| await php.mount( '/tmp/sqlite-command', createNodeFsMountHandler( getSqliteCommandPath() ) ); |
There was a problem hiding this comment.
Nit: Would it make sense to use an array and a map here?
There was a problem hiding this comment.
This is a matter of preference, IMO. I think the current approach reads clearly, and changing it to an array that we iterate over won't really make the code more compact
| () => php.exit(), | ||
| ]; | ||
| } catch ( error ) { | ||
| throw new Error( __( 'An error occurred while running the WP-CLI command.', 'wp-playground' ) ); |
There was a problem hiding this comment.
Should this be translated? If yes, do we need to use a domain?
ivan-ottinger
left a comment
There was a problem hiding this comment.
I haven't worked with PHP-WASM much yet, but the changes look good to me and the commands run well.
I have tested with several commands, including the ones that make network requests (e.g. wp plugin install) and all of them run as expected. 👍🏼
|
Thanks for the reviews, @wojtekn and @ivan-ottinger
I'll take a look at this in a follow-up PR |
Related issues
Proposed Changes
Instead of instantiating Playground CLI, use a "raw" PHP-WASM instance to execute WP-CLI commands. In my quick benchmarks, this shows significantly better performance (
wp option gettakes 1.93s on this branch compared to 4.94s on the base branch).This is essentially the same strategy that we already use on trunk. @bgrgicak very graciously helped me with some modifications to fix some issues we were having with network requests (
wp plugin installcommands would previously fail, for example).Testing Instructions
npm run cli:buildnode dist/cli/main.js wp --path PATH_TO_STUDIO_SITEPre-merge Checklist