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/node: Update express to newest version, and move it to devDependencies #1218

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

eliot-akira
Copy link
Collaborator

What is this PR doing?

This is the first part of solving WordPress/playground-tools#224 (wp-now: Resolve npm audit warnings).

  • Update express to newest 4.19.2
  • Move it to devDependencies
  • Remove unused express-fileupload and follow-redirects

What problem is it solving?

It updates express to the newest version with security vulnerabilities fixed.

How is the problem addressed?

In addition to updating it, it's moved to devDependencies because express is only used in tests.

./packages/php-wasm/node/src/test/php-networking.spec.ts:2:import express from 'express';

This consequently removes express from being installed when installing @php-wasm/node.

The PR also removes unnecessary dependencies, express-fileupload and follow-redirects, which are only used for wp-now in the Playground Tools project.

Testing Instructions

npm run test

@bgrgicak
Copy link
Collaborator

I'm having some timeout issue with tests passing locally, but it could be just my machine.

Let's wait with this PR until we fix the build (I'm working on it today) and can confirm all tests pass.

@adamziel
Copy link
Collaborator

The build and e2e should pass now. @eliot-akira would you rebase?

adamziel pushed a commit to WordPress/playground-tools that referenced this pull request Apr 12, 2024
#225)

This is the second part of solving:

- #224 

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](133029c).
…unused express-fileupload and follow-redirects
@eliot-akira
Copy link
Collaborator Author

OK, I rebased from trunk, force pushed, and tests are passing.

@adamziel adamziel merged commit 14559cd into WordPress:trunk Apr 15, 2024
5 checks passed
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