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.

Sign up for GitHub

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

Jump to bottom

Update dependencies and fix e2e tests #2160

Merged
merged 21 commits into from
Sep 19, 2024
Merged

Update dependencies and fix e2e tests #2160

merged 21 commits into from
Sep 19, 2024

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Sep 2, 2024 •
edited
Loading

This PR:

  • Bumps webpack from 5.831.1 to 5.94.0 (same as PR Bump webpack from 5.83.1 to 5.94.0 #2159).
  • Updates the version of Node to be in sync with WooCommerce.
  • I included validate-build.js to the excluded files from builds.
  • Updated several other packages that were necessary. That also involved:
    • Disabling a new ESLint rule: @typescript-eslint/no-this-alias.
    • Updating the only e2e test we have.
    • Rebuilding package-lock.json from scratch.

This allows us to get the e2e tests job pass again.

How to test the changes in this Pull Request:

This PR doesn't include any new features, it's simply updating dependencies and internal tools.

  1. Run npm ci && npm run build to build the ZIP.
  2. Install the generated ZIP in a fresh store.
  3. Do some smoke testing.
  4. Verify everything works as it used to and there are no regressions.

Changelog

Update many dev dependencies.

@dependabot
Bump webpack from 5.83.1 to 5.94.0
fe6202b 
Bumps [webpack](https://github.com/webpack/webpack) from 5.83.1 to 5.94.0.
- [Release notes](https://github.com/webpack/webpack/releases)
- [Commits](webpack/webpack@v5.83.1...v5.94.0)

---
updated-dependencies:
- dependency-name: webpack
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
@Aljullu Aljullu added status: needs review PR that needs review dependencies Pull requests that update a dependency file labels Sep 2, 2024
@Aljullu Aljullu self-assigned this Sep 2, 2024
@Aljullu Aljullu removed the status: needs review PR that needs review label Sep 2, 2024
@Aljullu Aljullu force-pushed the update/webpack-5.94.0 branch from 6daa1e6 to 713db2a Compare September 2, 2024 13:47
@Aljullu Aljullu force-pushed the update/webpack-5.94.0 branch from 99b6228 to 6decafd Compare September 3, 2024 10:11
@Aljullu Aljullu changed the title Bump webpack from 5.83.1 to 5.94.0 Update dependencies and fix e2e tests Sep 3, 2024
@Aljullu Aljullu requested review from a team and dinhtungdu and removed request for a team September 3, 2024 13:59
dinhtungdu
dinhtungdu reviewed Sep 18, 2024
View reviewed changes
Copy link
Member

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

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

Thanks for working on this PR and I'm sorry for the late reviews! I built and smoked test the zip file locally and the built theme works flawlessly in my test, no console log or PHP error thrown. I left some minor comments, otherwise, this is LGTM.

package.json Outdated
@@ -70,39 +60,50 @@
"STORE_URL": "http://localhost:8802"
}
},
"lorem": "ipsum",
Copy link
Member

Choose a reason for hiding this comment

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

What is this used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ups, a leftover from testing. Removed in 54f1178.

package.json Outdated
},
"engines": {
"node": "14.21.1",
"node": "^20.11.1",
"npm": "9.1.3"
Copy link
Member

Choose a reason for hiding this comment

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

Let's also pump the npm version too, as v10 is ship with node 20

Copy link
Member

Choose a reason for hiding this comment

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

https://nodejs.org/en/about/previous-releases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in ea5e7b8.

@Aljullu Aljullu force-pushed the update/webpack-5.94.0 branch from 81c44cc to ea5e7b8 Compare September 18, 2024 14:45
@Aljullu Aljullu requested a review from dinhtungdu September 18, 2024 14:51
@Aljullu
Copy link
Contributor Author

Aljullu commented Sep 18, 2024

Thanks for the review, @dinhtungdu! This is ready for another look.

dinhtungdu
dinhtungdu approved these changes Sep 19, 2024
View reviewed changes
Copy link
Member

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! I smoke-tested the build and it still worked as expected for me.

@github-actions github-actions bot added the status: ready to merge Automatically applied to a pull when a pull is approved. Indicates ready for merging. label Sep 19, 2024
@Aljullu Aljullu merged commit 7100254 into trunk Sep 19, 2024
8 checks passed
@Aljullu Aljullu deleted the update/webpack-5.94.0 branch September 19, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dinhtungdu dinhtungdu dinhtungdu approved these changes

Assignees

@Aljullu Aljullu

Labels
dependencies Pull requests that update a dependency file status: ready to merge Automatically applied to a pull when a pull is approved. Indicates ready for merging.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Aljullu @dinhtungdu