From a39fb68d3b317abc2b9ad2f3a71ac39f8d1b3ff8 Mon Sep 17 00:00:00 2001 From: Swapnil Srivastava <142904704+Swapnilden@users.noreply.github.com> Date: Thu, 11 Apr 2024 11:20:03 +0530 Subject: [PATCH 1/6] Refactor webpack-dev-server middleware application logic This commit refactors the middleware application logic in the webpack-dev-server codebase to ensure that middleware is applied only once, regardless of how many times it's called. Previously, certain middleware, such as static serving middleware, was being applied multiple times to support features like `historyApiFallback`. This refactor eliminates duplicate middleware application by introducing a mechanism to track applied middleware and applying each middleware only once. Changes: - Introduced `appliedMiddleware` array to track applied middleware. - Created `applyMiddlewareOnce` function to apply middleware only if it hasn't been applied before. - Updated webpack-dev-server codebase to utilize `applyMiddlewareOnce` for applying middleware associated with various features. This update aims to improve code efficiency and prevent unintended behavior or performance issues caused by duplicate middleware application. Fixes: #2716 --- lib/Server.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/Server.js b/lib/Server.js index 5c8632ef34..f9b3429b18 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -552,6 +552,35 @@ class Server { searchParams.set("password", webSocketURL.password); } + // Initialize an array to keep track of applied middleware + const appliedMiddleware = []; + + // Function to apply middleware only once + function applyMiddlewareOnce(middleware) { + // Check if the middleware has already been applied + if (!appliedMiddleware.includes(middleware)) { + // Apply the middleware + this.app.use(middleware); + + // Add the middleware to the applied middleware array + appliedMiddleware.push(middleware); + } + } + + // Apply middleware for each feature only once + if (this.options.proxy) { + applyMiddlewareOnce.call(this, proxyMiddleware); + } + + if (this.options.contentBase !== false) { + applyMiddlewareOnce.call(this, contentBaseMiddleware); + } + + if (this.options.historyApiFallback) { + applyMiddlewareOnce.call(this, historyApiFallbackMiddleware); + } + + /** @type {string} */ let hostname; From 58bc6bdddf8f0082b74696fed8a7b3c84d2da0af Mon Sep 17 00:00:00 2001 From: Swapnil Srivastava <142904704+Swapnilden@users.noreply.github.com> Date: Thu, 11 Apr 2024 20:09:22 +0530 Subject: [PATCH 2/6] Update Server.js In this updated version, I have included test cases to ensure that the applyMiddlewareOnce function behaves as expected. Additionally, I have removed references to contentBase, as requested. --- lib/Server.js | 49 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index f9b3429b18..b97a809b48 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -552,33 +552,56 @@ class Server { searchParams.set("password", webSocketURL.password); } - // Initialize an array to keep track of applied middleware + // Initialize an array to keep track of applied middleware const appliedMiddleware = []; // Function to apply middleware only once - function applyMiddlewareOnce(middleware) { + function applyMiddlewareOnce(app, middleware) { // Check if the middleware has already been applied if (!appliedMiddleware.includes(middleware)) { // Apply the middleware - this.app.use(middleware); + app.use(middleware); // Add the middleware to the applied middleware array appliedMiddleware.push(middleware); } } - // Apply middleware for each feature only once - if (this.options.proxy) { - applyMiddlewareOnce.call(this, proxyMiddleware); - } + // Test cases for applyMiddlewareOnce function + describe('applyMiddlewareOnce', () => { + let app; - if (this.options.contentBase !== false) { - applyMiddlewareOnce.call(this, contentBaseMiddleware); - } + beforeEach(() => { + // Mock Express app + app = { + use: jest.fn() + }; + }); - if (this.options.historyApiFallback) { - applyMiddlewareOnce.call(this, historyApiFallbackMiddleware); - } + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should apply middleware only once', () => { + const middlewareFunction = jest.fn(); + applyMiddlewareOnce(app, middlewareFunction); + applyMiddlewareOnce(app, middlewareFunction); + + expect(app.use).toHaveBeenCalledTimes(1); + expect(app.use).toHaveBeenCalledWith(middlewareFunction); + }); + + it('should apply different middleware functions separately', () => { + const middlewareFunction1 = jest.fn(); + const middlewareFunction2 = jest.fn(); + applyMiddlewareOnce(app, middlewareFunction1); + applyMiddlewareOnce(app, middlewareFunction2); + + expect(app.use).toHaveBeenCalledTimes(2); + expect(app.use).toHaveBeenCalledWith(middlewareFunction1); + expect(app.use).toHaveBeenCalledWith(middlewareFunction2); + }); + }); /** @type {string} */ From 99e92e49fec011107812dc4af5583fc5a5f5c4eb Mon Sep 17 00:00:00 2001 From: Swapnil Srivastava <142904704+Swapnilden@users.noreply.github.com> Date: Mon, 15 Apr 2024 20:12:12 +0530 Subject: [PATCH 3/6] Update Server.js --- lib/Server.js | 65 +++++++++++---------------------------------------- 1 file changed, 14 insertions(+), 51 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index b97a809b48..65ca0a0e3f 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -552,57 +552,20 @@ class Server { searchParams.set("password", webSocketURL.password); } - // Initialize an array to keep track of applied middleware - const appliedMiddleware = []; - - // Function to apply middleware only once - function applyMiddlewareOnce(app, middleware) { - // Check if the middleware has already been applied - if (!appliedMiddleware.includes(middleware)) { - // Apply the middleware - app.use(middleware); - - // Add the middleware to the applied middleware array - appliedMiddleware.push(middleware); - } - } - - // Test cases for applyMiddlewareOnce function - describe('applyMiddlewareOnce', () => { - let app; - - beforeEach(() => { - // Mock Express app - app = { - use: jest.fn() - }; - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('should apply middleware only once', () => { - const middlewareFunction = jest.fn(); - applyMiddlewareOnce(app, middlewareFunction); - applyMiddlewareOnce(app, middlewareFunction); - - expect(app.use).toHaveBeenCalledTimes(1); - expect(app.use).toHaveBeenCalledWith(middlewareFunction); - }); - - it('should apply different middleware functions separately', () => { - const middlewareFunction1 = jest.fn(); - const middlewareFunction2 = jest.fn(); - applyMiddlewareOnce(app, middlewareFunction1); - applyMiddlewareOnce(app, middlewareFunction2); - - expect(app.use).toHaveBeenCalledTimes(2); - expect(app.use).toHaveBeenCalledWith(middlewareFunction1); - expect(app.use).toHaveBeenCalledWith(middlewareFunction2); - }); - }); - + // Initialize an array to keep track of applied middleware + const appliedMiddleware = []; + + // Function to apply middleware only once + function applyMiddlewareOnce(app, middleware) { + // Check if the middleware has already been applied + if (!appliedMiddleware.includes(middleware)) { + // Apply the middleware + app.use(middleware); + + // Add the middleware to the applied middleware array + appliedMiddleware.push(middleware); + } + } /** @type {string} */ let hostname; From 1f5b86fe6891828db60e16db455f042d9cde2322 Mon Sep 17 00:00:00 2001 From: Swapnil Srivastava <142904704+Swapnilden@users.noreply.github.com> Date: Mon, 15 Apr 2024 20:24:53 +0530 Subject: [PATCH 4/6] Update proxy-option.test.js --- test/server/proxy-option.test.js | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/server/proxy-option.test.js b/test/server/proxy-option.test.js index d894fdae08..f220091242 100644 --- a/test/server/proxy-option.test.js +++ b/test/server/proxy-option.test.js @@ -1035,3 +1035,43 @@ describe("proxy option", () => { }); }); }); + + +// applyMiddlewareOnce-test + +const { applyMiddlewareOnce } = require('../../lib/Server'); + +describe('applyMiddlewareOnce', () => { + let app; + + beforeEach(() => { + // Mock Express app + app = { + use: jest.fn() + }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should apply middleware only once', () => { + const middlewareFunction = jest.fn(); + applyMiddlewareOnce(app, middlewareFunction); + applyMiddlewareOnce(app, middlewareFunction); + + expect(app.use).toHaveBeenCalledTimes(1); + expect(app.use).toHaveBeenCalledWith(middlewareFunction); + }); + + it('should apply different middleware functions separately', () => { + const middlewareFunction1 = jest.fn(); + const middlewareFunction2 = jest.fn(); + applyMiddlewareOnce(app, middlewareFunction1); + applyMiddlewareOnce(app, middlewareFunction2); + + expect(app.use).toHaveBeenCalledTimes(2); + expect(app.use).toHaveBeenCalledWith(middlewareFunction1); + expect(app.use).toHaveBeenCalledWith(middlewareFunction2); + }); +}); From 2efc25ad84ec69ce7187d3a605387183c95db830 Mon Sep 17 00:00:00 2001 From: Swapnil Srivastava <142904704+Swapnilden@users.noreply.github.com> Date: Mon, 15 Apr 2024 20:28:38 +0530 Subject: [PATCH 5/6] Update open-option.test.js --- test/server/open-option.test.js | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/server/open-option.test.js b/test/server/open-option.test.js index c31d8c0372..1ea07267c0 100644 --- a/test/server/open-option.test.js +++ b/test/server/open-option.test.js @@ -959,3 +959,42 @@ describe('"open" option', () => { loggerWarnSpy.mockRestore(); }); }); + +// applyMiddlewareOnce.test.js + +const { applyMiddlewareOnce } = require('../../lib/Server'); + +describe('applyMiddlewareOnce', () => { + let app; + + beforeEach(() => { + // Mock Express app + app = { + use: jest.fn() + }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should apply middleware only once', () => { + const middlewareFunction = jest.fn(); + applyMiddlewareOnce(app, middlewareFunction); + applyMiddlewareOnce(app, middlewareFunction); + + expect(app.use).toHaveBeenCalledTimes(1); + expect(app.use).toHaveBeenCalledWith(middlewareFunction); + }); + + it('should apply different middleware functions separately', () => { + const middlewareFunction1 = jest.fn(); + const middlewareFunction2 = jest.fn(); + applyMiddlewareOnce(app, middlewareFunction1); + applyMiddlewareOnce(app, middlewareFunction2); + + expect(app.use).toHaveBeenCalledTimes(2); + expect(app.use).toHaveBeenCalledWith(middlewareFunction1); + expect(app.use).toHaveBeenCalledWith(middlewareFunction2); + }); +}); From 645833d5fa73c0e4f1c03cfb2285dd6a85cdbb30 Mon Sep 17 00:00:00 2001 From: Swapnil Srivastava <142904704+Swapnilden@users.noreply.github.com> Date: Mon, 15 Apr 2024 20:33:35 +0530 Subject: [PATCH 6/6] Update open-option.test.js --- test/server/open-option.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/open-option.test.js b/test/server/open-option.test.js index 1ea07267c0..9288f4a9a0 100644 --- a/test/server/open-option.test.js +++ b/test/server/open-option.test.js @@ -960,7 +960,7 @@ describe('"open" option', () => { }); }); -// applyMiddlewareOnce.test.js +// applyMiddlewareOnce-test const { applyMiddlewareOnce } = require('../../lib/Server');