fix: Upgrade webpack dev server #39566
Conversation
riodeuno
added
the
Security
WalkthroughThis pull request removes the HTTPS configuration from the Webpack Dev Server setup and replaces the Changes
Sequence Diagram(s)sequenceDiagram
participant DevServer as Development Server
participant SW as setupMiddlewares
participant ESM as evalSourceMapMiddleware
participant RS as redirectServedPath
participant NSWM as noopServiceWorkerMiddleware
DevServer->>SW: invoke setupMiddlewares(middlewares, devServer)
SW->>ESM: unshift (add first)
SW->>RS: push (append)
SW->>NSWM: push (append)
SW-->>DevServer: return middlewares array
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@riodeuno Fix the following issue: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/config/webpackDevServer.config.js (1)
9-9: Potential unused import after configuration changes.The
getHttpsConfigimport is still present but appears to be unused after removing the HTTPS configuration.-const getHttpsConfig = require("./getHttpsConfig");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
app/client/config/webpackDevServer.config.js(1 hunks)app/client/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
🔇 Additional comments (3)
app/client/package.json (1)
395-395:❓ Verification inconclusive
Major version upgrade of webpack-dev-server requires careful validation.
The upgrade from webpack-dev-server v4 to v5 involves significant API changes. This aligns with the modifications in the webpack configuration file.
🏁 Script executed:
#!/bin/bash # Check for any breaking changes or migration guides for webpack-dev-server v5 curl -s https://www.npmjs.com/package/webpack-dev-server | grep -A 10 "Breaking Changes" # Check if there are any remaining references to old webpack-dev-server API rg "onBeforeSetupMiddleware|onAfterSetupMiddleware" --type jsLength of output: 154
Attention: Manual Verification Needed for webpack-dev-server v5 Upgrade
The migration to webpack-dev-server v5 involves major API changes, especially in how middleware is configured. Although our automated search didn’t reveal any lingering legacy hooks likeonBeforeSetupMiddlewareoronAfterSetupMiddleware, please manually verify that your webpack configuration fully adheres to the updated API (for example, by ensuring that the newsetupMiddlewaresapproach is correctly implemented).
- Double-check all webpack configuration files to ensure deprecated hooks are completely removed.
- Confirm that the configuration has been updated per the official webpack-dev-server v5 migration guide.
app/client/config/webpackDevServer.config.js (2)
100-117: Middleware configuration updated to match webpack-dev-server v5 API.The previous hooks
onBeforeSetupMiddlewareandonAfterSetupMiddlewarehave been replaced with the unifiedsetupMiddlewaresmethod, which is the correct approach for webpack-dev-server v5. The middleware logic itself remains functionally equivalent.
100-100:✅ Verification successful
HTTPS configuration has been removed.
The HTTPS configuration has been removed from the webpack dev server setup. Ensure this is intentional and that developers are aware HTTPS will no longer be available in the development environment.
🏁 Script executed:
#!/bin/bash # Check if HTTPS is used elsewhere in the codebase for local development rg "HTTPS=true|https:.*true" --type js --type json # Check if there are any docs or README files mentioning HTTPS for local development rg "HTTPS.*development|secure.*localhost" --type mdLength of output: 65964
Action: Confirm intentional removal of HTTPS configuration from the webpack dev server
The HTTPS configuration has been removed as expected. I searched the codebase for any local development flags or settings (e.g., "HTTPS=true" or similar references) and found that while some tests assert that URLs start with "https://docs.appsmith.com/", they only rely on external documentation links rather than on a local HTTPS setup. There aren’t any indications elsewhere that the dev server depends on an HTTPS configuration.
- Verify that no local development logic relies on webpack’s HTTPS configuration.
- Note that tests referencing https URLs (e.g., in Cypress specs) are unaffected by this removal.
This removal appears intentional. Please confirm that relying solely on HTTP for local development is acceptable for your use case.
Description
Update webpack dev server to update path-to-regexp to v0.1.12
Fixes https://github.com/appsmithorg/appsmith/security/dependabot/360
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13673398290
Commit: 1bc2243
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts
List of identified flaky tests.Wed, 05 Mar 2025 11:18:55 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Refactor
Chores