Skip to content

Conversation

@wjrosa
Copy link
Contributor

@wjrosa wjrosa commented Sep 1, 2025

Fixes STRIPE-696
Base PR #4264

Changes proposed in this Pull Request:

As part of the Node upgrade project, this PR upgrades additional NPM packages to match the main PR: cross-env, mkdirp, and rimraf.

  • cross-env: allows usage of environment variables in commands
  • mkdirp: directory creation
  • rimraf: file deletion

We are also removing the chromedriver package, as we don't need it.

Testing instructions

  • Checkout and build this branch on your test environment (dev/upgrading-additional-packages)
  • Check if the frontend tests are still passing

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

@wjrosa wjrosa self-assigned this Sep 1, 2025
@wjrosa wjrosa marked this pull request as ready for review September 1, 2025 20:19
@wjrosa
Copy link
Contributor Author

wjrosa commented Sep 1, 2025

Hey @diegocurbelo! Would you remember why it was necessary to add the ajv and wait-on packages to the main PR? Just double-checking if they are really necessary.

@wjrosa wjrosa requested review from a team, diegocurbelo and malithsen and removed request for a team September 1, 2025 20:22
@wjrosa
Copy link
Contributor Author

wjrosa commented Sep 3, 2025

I went ahead and removed both packages. The main PR works just fine. So I think it is safe to remove them.

@diegocurbelo
Copy link
Member

Hey @diegocurbelo! Would you remember why it was necessary to add the ajv and wait-on packages to the main PR? Just double-checking if they are really necessary.

It was because a couple of packages used different (and incompatible) versions of those two dependencies, and the npm resolver failed to find a version that worked for all of them; it might not be needed anymore.

Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

The updated versions work, tests pass, and Woorelease generated the package correctly (including the build folder); both flows use rimraf and cross-env.

chromedriver: for e2e tests

We are using Playwright for the E2E tests, which does not require chromedriver.

@wjrosa
Copy link
Contributor Author

wjrosa commented Sep 4, 2025

That's good to know, Diego! Thank you. I removed the chromedriver package in 5c9f161. And everything seems to be working just fine indeed.

Copy link
Contributor

@daledupreez daledupreez left a comment

Choose a reason for hiding this comment

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

The changes LGTM, and both e2e and JS tests are looking good. :shipit:

Co-authored-by: daledupreez <[email protected]>
@wjrosa wjrosa enabled auto-merge (squash) September 4, 2025 15:42
@wjrosa wjrosa disabled auto-merge September 4, 2025 15:56
@wjrosa wjrosa merged commit 4a0eadb into develop Sep 4, 2025
39 of 40 checks passed
@wjrosa wjrosa deleted the dev/upgrading-additional-packages branch September 4, 2025 15:57
@daledupreez daledupreez added this to the 10.0.0 milestone Oct 8, 2025
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.

4 participants