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

Add express to dependencies; Update follow-redirects to newest version #225

Merged
merged 2 commits into from
Apr 12, 2024

Conversation

eliot-akira
Copy link
Contributor

@eliot-akira eliot-akira commented Apr 9, 2024

What?

This is the second part of solving:

The first part is WordPress/wordpress-playground#1218. (php-wasm/node: Update express to newest version, and move it to devDependencies)

Why?

The first part of the solution is in the Playground project, which updates express and moves it to devDependencies since it's only used for tests in that project.

As a result, express will no longer be installed when @php-wasm/node is installed. So wp-now needs to declare express as a dependency.

How?

  • Add express to dependencies
  • Update follow-redirects to newest 1.15.6

These together with WordPress/wordpress-playground#1218 will resolve the original issue #224, which should eliminate all npm audit warnings.

Testing Instructions

Currently npm run test fails with an unrelated issue, as can be seen in another PR's CI test run. https://github.com/WordPress/playground-tools/actions/runs/8616282863/job/23614052137?pr=223#step:4:49 ("RuntimeError: memory access out of bounds") That seems have been caused by commit 133029c.

@sejas
Copy link
Collaborator

sejas commented Apr 10, 2024

@eliot-akira , thanks for this PR.

If you pull the latest changes from trunk, the tests should pass.

@eliot-akira
Copy link
Contributor Author

eliot-akira commented Apr 10, 2024

OK, merged from trunk, ran npm install, and now the tests pass locally and in the CI.

I suppose this PR can be merged before the other one (WordPress/wordpress-playground#1218), since that removes the express dependency from @php-wasm/node.

As an aside, recently I'm integrating wp-now in a team project setup, to make onboarding new members simpler by replacing wp-env and Docker. So far it's working great, much faster to get started.

@adamziel adamziel merged commit d78c044 into WordPress:trunk Apr 12, 2024
2 checks passed
@adamziel
Copy link
Collaborator

As an aside, recently I'm integrating wp-now in a team project setup, to make onboarding new members simpler by replacing wp-env and Docker. So far it's working great, much faster to get started.

@eliot-akira YAY! I'm so glad to hear that! Personally I've ran into some rough edges with modes and auto-mounting things and would love to make that optional and configurable via CLI options. But other than it did the trick on my end, too. I think you'd like Studio@sejas could give you early access.

@eliot-akira
Copy link
Contributor Author

eliot-akira commented Apr 12, 2024

Studio looks cool! From the screenshot I imagine a desktop app for managing local WordPress sites powered by Playground. Curious to try it, I signed up for the waitlist.

I'm still solving a few things to completely replace wp-env with wp-now.

  • The "mapping" feature of .wp-env.json, a declarative way to mount local folders to the virtual file system. Surely it can be achieved with blueprints, or a script that reads an existing env file and generates dynamic blueprint steps.
  • Run PHPUnit tests using wp-now. Getting close but not there yet (ran into WASM error, something to do with Asyncify.handleSleep - will file an issue when I have a minimal reproducible example).

@adamziel
Copy link
Collaborator

@eliot-akira for mapping, let's explore that in Blueprints v2:

WordPress/blueprints-library#46

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.

3 participants