Fix: Update webpack-dev-server to ^5.2.1 to help with dep alerts#80347
Fix: Update webpack-dev-server to ^5.2.1 to help with dep alerts#80347im3dabasia wants to merge 2 commits into
Conversation
|
Since this is a major bump. Did we check if there are any breaking changes that might affect us or the consumers? |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Went through the v5 migration guide line by line against our config in The one item that does need attention: the proxy option's schema changed from an object map to an array-only format. Our config currently uses the old object shape: Reference to migration guide by webpack team proxy: {
'/build': { pathRewrite: { '^/build': '' } },
},Per the guide, this needs to be: proxy: [
{ context: ['/build'], pathRewrite: { '^/build': '' } },
],This is the only config-shape change this bump actually requires on our end. |
|
While updating the proxy syntax above, I wanted to actually verify it works rather than just match the guide's shape, so I scaffolded a test plugin with Found that So this isn't a regression from this PR, it's a pre-existing bug in the current config that just hadn't been noticed. Root cause: the proxy object was never given a target. Adding one fixes it: proxy: [
{
context: ['/build'],
target: 'http://localhost:8887',
pathRewrite: { '^/build': '' },
},
],Confirmed with Since this bug predates this PR and isn't caused by the version bump, I don't think it needs to block this PR, but flagging it here since it surfaced directly from testing this change, might be worth a follow-up issue/PR to add the missing target. |
manzoorwanijk
left a comment
There was a problem hiding this comment.
Looks good. Thank you for checking the breaking changes.
Thanks, @manzoorwanijk, for the review: Based on my findings(#80347 (comment)), I think that to use the proxy correctly, we need to pass a target to the proxy object here. Should I go ahead and open a PR for this? This bug has likely remained unnoticed for a long time since I don't think this functionality is used very often. Here is the official reference for the same: https://webpack.js.org/configuration/dev-server/#devserverproxy
|
Yes please. Thank you. |
What?
Closes the following dep alerts
https://github.com/WordPress/gutenberg/security/dependabot/384
https://github.com/WordPress/gutenberg/security/dependabot/383
https://github.com/WordPress/gutenberg/security/dependabot/385
https://github.com/WordPress/gutenberg/security/dependabot/382
https://github.com/WordPress/gutenberg/security/dependabot/651
https://github.com/WordPress/gutenberg/security/dependabot/650
https://github.com/WordPress/gutenberg/security/dependabot/692
https://github.com/WordPress/gutenberg/security/dependabot/691
Updates the webpack-dev-server to patched version to solve the dep alerts and make the package malware free.
Why?
Security enhancements for the package
How?
Updated the package in
packages/scripts/package.jsonto use version^5.2.1which is patched version for the malware.Screenshots or screencast
After