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

WP-NOW: Add wp-cli integration #212

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

WP-NOW: Add wp-cli integration #212

wants to merge 4 commits into from

Conversation

sejas
Copy link
Collaborator

@sejas sejas commented Apr 3, 2024

What?

Brings wp-cli execution to wp-now with the current site context.
The new command will be wp-now wp ...
I'm open to other command names, like cli.

Why?

wp-cli is a handy tool for all WordPress developers opening a wide range of automatizations.

How?

wp-cli was intentionally removed until the Playground was mature enough to support a wide range of operations.
I added a new command that calls executeWPCli, which downloads the phar file and mounts the current directory to make easier the execution of local files with eval-file.

Testing Instructions

  1. Run nvm use && npm install && npx nx build wp-now
  2. Run npx nx run wp-now:test
  3. Observe the tests pass
  4. Run node dist/packages/wp-now/cli.js wp eval-file /./foo.php
  5. Run node dist/packages/wp-now/cli.js wp cli version
  6. Run other wp-cli commands
  7. Confirm wp-cli runs as expected

@sejas sejas self-assigned this Apr 3, 2024
@kozer
Copy link
Collaborator

kozer commented Apr 3, 2024

I tested other commands, and on the first one I got an unhandled exception:

❯ node dist/packages/wp-now/cli.js wp cli info
OS:     Emscripten 3.1.43 #1 wasm32
Shell:
PHP binary:
PHP version:    8.0.30-dev
php.ini used:
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "ErrnoError: FS error".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

Node.js v18.18.2

Not sure if I needed to pass another argument to make it work, however, for sure the unhandled exception should not be happening. Correct?

},
async () => {
// 0: node, 1: wp-now, 2: wp, 3: [wp-cli options...]
const args = process.argv.slice(3);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Can we replace the 3 with something that express better your comment?
Eg:

const args = {
    node: 0,
    wpNow: 1,
    wp: 2,
    wpCliOptions: 3
} 

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't yargs provide you a list of everything that comes after the command?

* Test wp-cli eval-file works correctly.
* We will use the context of Playground mode.
*/
test('wp-cli eval-file works correctly', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about that command, how lovely!

@n3f
Copy link

n3f commented Jul 28, 2024

wp cli info fails for me too, but it seems to be more of a problem with the PHP wasm implementation than wp-now. (At least I couldn't find a way to get it to successfully trap the exception.

@adamziel
Copy link
Collaborator

@n3f what would be a reproduction?

@n3f
Copy link

n3f commented Jul 31, 2024

@n3f what would be a reproduction?

I just followed the instructions in the PR then as an additional case ran node dist/packages/wp-now/cli.js wp cli info (just like @kozer). I saw the same error, but I looked to see if there was a way to catch it, but it appeared to be caught and handled at the php boundary -- but I might just not understand how it works correctly.

@sejas
Copy link
Collaborator Author

sejas commented Aug 1, 2024

@n3f , thanks for testing it.
Yeah, not sure why wp cli info fails in php-wasm/node version and it doesn't in the web version https://playground.wordpress.net/demos/wp-cli.html

Maybe it's a matter of using the latest dependencies.

Screenshot 2024-08-01 at 10 37 59

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wp-now: Make it possible to use WP-CLI
4 participants