fix: devServer put static serve proxy after umi proxy middleware

[whyer11]

Fix the issue where the devServer does not correctly return HTML type resources, and add related e2e test cases for umijs devServer.

Summary by CodeRabbit

• 新功能

  • 增强了开发服务器的日志记录功能，提供更好的请求跟踪。
  • 新增开发环境配置文件，允许开发者灵活设置应用运行端口。
  • 引入新的异步函数以验证本地开发服务器的响应。
  • 新增样式文件，改善应用的外观。
  • 改进了脚本的控制流，动态处理服务器状态，确保更好的错误管理。
  • 修改了开发服务器关闭函数，提升了端口检查的灵活性和错误报告的清晰度。

• 错误修复

  • 修正了测试函数的命名错误，提升了代码的可读性。

[coderabbitai]

Warning

Rate limit exceeded

@stormslowly has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 46 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 9d6bdec and b804d6b.

Walkthrough

此次更改涉及多个文件，主要集中在开发服务器的请求处理、测试功能的增强以及配置文件的引入。具体而言，开发服务器的静态内容处理逻辑得到了改进，测试模块增加了对服务器状态的检查功能，同时新增了环境配置文件、样式文件和异步测试函数，以支持开发环境的灵活性和可维护性。

Changes

文件路径	更改摘要
crates/mako/src/dev.rs	改进了静态内容服务的控制流，增加了请求的调试日志。
e2e/fixtures.umi/dev.hmr/.env	新增了用于开发环境的配置文件，定义了 PORT 变量。
e2e/fixtures.umi/dev.hmr/expect.js	新增了一组异步函数，用于测试开发服务器的功能。
e2e/fixtures.umi/dev.hmr/global.css	新增了 CSS 文件，定义了 body 元素的文本颜色为红色。
packages/bundler-mako/index.js	移除了 processReqURL 函数，新增了基于请求方法和内容类型的中间件，简化了请求处理逻辑。
scripts/test-e2e.mjs	增强了服务器连接检查功能，修改了控制流以动态处理服务器状态。
scripts/test-hmr.mjs	修改了 killMakoDevServer 函数，允许动态指定端口，增强了函数的灵活性和错误报告。

Possibly related PRs

• fix: dev server support "publicPath" #1398: 该PR涉及开发服务器的实现和静态内容处理，相关于本PR的请求处理逻辑改进。
• refactor: Unify the static server in bundler-mako and devServer #1468: 该PR重构了静态服务器逻辑，与本PR在静态内容服务方面的更改直接相关。
• Revert "refactor: Unify the static server in bundler-mako and devServer" #1556: 该PR恢复了之前重构中对静态服务器的更改，表明静态内容处理是一个重要关注点。
• fix: When hmr=none,mako does not take effect #1552: 该PR修改了 hmr 配置的处理，可能间接关系到本PR讨论的服务器行为和请求处理。

🐰 在草地上跳跃，变化真美妙，
代码如花绽放，功能更高效。
服务器连接好，测试不再愁，
新增的样式亮，开发乐逍遥！
兔子欢呼跳跃，庆祝这变化，
代码如春风，带来新希望！ 🌼

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 7

Outside diff range, codebase verification and nitpick comments (5)

e2e/fixtures.umi/dev.hmr/.umirc.ts (1)

1-14: 建议添加注释以提高可读性。

为了提高代码的可读性和可维护性，建议为每个配置项添加简短的注释，解释其用途和影响。

建议添加如下注释：

 export default {
+  // 禁用 MFSU（Module Federation Speed Up）
   mfsu: false,
+  // Mako 配置（暂时为空）
   mako: {},
+  // Less 加载器配置
   lessLoader: {
+    // 始终计算数学表达式
     math: 'always',
   },
+  // 代理配置
   proxy: {
     '/api': {
+      // 目标服务器地址
       'target': 'http://jsonplaceholder.typicode.com/',
+      // 修改请求头中的 host 值
       'changeOrigin': true,
+      // 重写路径
       'pathRewrite': { '^/api' : '' },
     }
   }
 };

e2e/fixtures.umi/dev.hmr/expect.mjs (1)

1-37: 测试套件结构良好，但可以进一步改进。

这个测试套件为不同类型的资源提供了良好的基本覆盖。然而，我们可以通过以下方式使其更加全面和结构化：

1. 使用测试框架：考虑使用像 Jest 这样的测试框架，它提供更好的结构化和报告功能。

2. 分组测试：将相关的测试分组到描述性的块中，例如：

describe('UmiJS 开发服务器测试', () => {
  test('首页应正确响应', async () => {
    // fetchHome 逻辑
  });

  test('API 应返回正确的 JSON', async () => {
    // fetchApi 逻辑
  });

  // 其他测试...
});

3. 添加更多边缘情况：测试无效路由、大文件、并发请求等。

4. 环境设置：添加 beforeAll 和 afterAll 钩子来启动和关闭服务器。

5. 配置测试：允许通过环境变量或配置文件更改端口等设置。

这些改进将使测试套件更加健壮和可维护。

scripts/test-e2e.mjs (2)

6-18: checkServer 函数实现良好

checkServer 函数的实现是合理的，它正确地使用了 net 模块来检查服务器连接状态。

建议考虑添加一个超时机制，以防止在某些情况下连接挂起太长时间：

 function checkServer(port, host, callback) {
   const client = new net.Socket();
+  const timeout = 5000; // 5秒超时
+
+  const timer = setTimeout(() => {
+    client.destroy();
+    callback(false);
+  }, timeout);

   client.connect({ port, host }, () => {
+    clearTimeout(timer);
     client.end();
     callback(true);
   });

   client.on('error', () => {
+    clearTimeout(timer);
     client.destroy();
     callback(false);
   });
 }

---

Line range hint 1-131: 总体评价：显著改进，但仍有优化空间

这次更改大大增强了 scripts/test-e2e.mjs 文件的功能和灵活性。主要改进包括：

1. 新增了服务器连接检查功能。
2. 增加了处理不同类型测试（dev 和 build）的能力。
3. 改进了期望（expect）文件的导入和执行逻辑。

这些改进使得测试脚本更加健壮和通用。然而，仍有一些优化空间：

1. 错误处理：在整个脚本中增加更详细的错误处理和日志记录。
2. 代码结构：将一些重复的逻辑抽取为独立的函数，以提高可维护性。
3. 配置管理：考虑将硬编码的值（如端口号、超时时间等）移至配置文件。
4. 测试覆盖：为新增的函数（如 checkServer、waitForServer）添加单元测试。

总的来说，这些更改是积极的，显著提升了脚本的功能。通过实施上述建议，可以进一步提高代码质量和可维护性。

crates/mako/src/dev.rs (1)

203-207: 改进的静态文件服务日志

修改后的调试日志消息现在包含了正在服务的文件路径，这提供了更多上下文信息，有助于跟踪哪些静态文件正在被服务。

为了使日志消息更加清晰，建议稍微调整一下格式：

debug!("Serving static file: {}", path);

这样可以更明确地表示正在执行的操作。

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 025591a and 1ad73b1.

Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

Files selected for processing (9)

• crates/mako/src/dev.rs (3 hunks)
• crates/mako/src/visitors/env_replacer.rs (1 hunks)
• e2e/fixtures.umi/dev.hmr/.env (1 hunks)
• e2e/fixtures.umi/dev.hmr/.umirc.ts (1 hunks)
• e2e/fixtures.umi/dev.hmr/expect.mjs (1 hunks)
• e2e/fixtures.umi/dev.hmr/pages/index.less (1 hunks)
• e2e/fixtures.umi/dev.hmr/pages/index.tsx (1 hunks)
• packages/bundler-mako/index.js (2 hunks)
• scripts/test-e2e.mjs (4 hunks)

Files skipped from review due to trivial changes (4)

• crates/mako/src/visitors/env_replacer.rs
• e2e/fixtures.umi/dev.hmr/.env
• e2e/fixtures.umi/dev.hmr/pages/index.less
• e2e/fixtures.umi/dev.hmr/pages/index.tsx

Additional comments not posted (6)

e2e/fixtures.umi/dev.hmr/.umirc.ts (2)

1-14: 配置文件结构正确，符合UmiJS标准。

整体配置结构看起来符合UmiJS的标准，包含了MFSU、Mako、Less加载器和代理设置。

---

7-13: 验证代理配置的正确性和安全性。

代理配置将'/api'路由转发到外部服务。请确保这是预期的行为，并验证目标服务器的安全性。

运行以下脚本以验证代理配置：

e2e/fixtures.umi/dev.hmr/expect.mjs (1)

1-4: 导入语句和端口定义看起来不错。

导入语句和端口定义适合此测试文件的目的。

scripts/test-e2e.mjs (1)

1-1: 导入 'net' 模块是合适的

新增的 'net' 模块导入是必要的，用于实现服务器连接检查功能。

crates/mako/src/dev.rs (1)

124-125: 新增的调试日志有助于请求跟踪

这个新增的调试日志语句可以记录传入请求的 HTTP 方法和 URI 路径，这对于调试和监控服务器行为非常有帮助。

packages/bundler-mako/index.js (1)

149-176: 新增中间件以优化请求处理逻辑

这个新增的中间件主要用于处理非HTML内容的GET和HEAD请求，通过创建代理到本地开发服务器来优化请求处理流程。

优点：

1. 简化了请求处理逻辑，移除了之前的processReqURL函数。
2. 通过检查请求方法和内容类型，更精确地控制了代理行为。
3. 引入了错误处理机制，提高了系统的健壮性。

建议：

1. 考虑将代理创建逻辑抽取为一个单独的函数，以提高代码的可读性和可维护性。
2. 确保这个更改不会影响到其他类型的请求处理，特别是HTML请求的处理逻辑。

+ function createDevServerProxy(hmrPort) {
+   return createProxyMiddleware({
+     target: `http://127.0.0.1:${hmrPort}`,
+     selfHandleResponse: true,
+     onProxyRes: (proxyRes, req, res) => {
+       if (proxyRes.statusCode !== 200) {
+         next();
+       } else {
+         proxyRes.pipe(res);
+       }
+     },
+     onError: (err, req, res) => {
+       next();
+     },
+   });
+ }

  app.use((req, res, next) => {
    if (req.method !== 'GET' && req.method !== 'HEAD') {
      return next();
    }

    if (req.accepts().join('').includes('html')) {
      return next();
    }

-   const devServerProxy = createProxyMiddleware({
-     target: `http://127.0.0.1:${hmrPort}`,
-     selfHandleResponse: true,
-     onProxyRes: (proxyRes, req, res) => {
-       if (proxyRes.statusCode !== 200) {
-         next();
-       } else {
-         proxyRes.pipe(res);
-       }
-     },
-     onError: (err, req, res) => {
-       next();
-     },
-   });
+   const devServerProxy = createDevServerProxy(hmrPort);
    devServerProxy(req, res, () => {
      next();
    });
  });

请运行以下脚本以验证HTML请求的处理是否受到影响：

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.11%. Comparing base (910dd76) to head (b804d6b).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1558   +/-   ##
=======================================
  Coverage   62.11%   62.11%           
=======================================
  Files         129      129           
  Lines       15559    15559           
=======================================
  Hits         9665     9665           
  Misses       5894     5894           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[whyer11]

上一个版本想复用wsProxy，但是qiankun这种请求子应用的方式是使用XHR，bundler中无法判断这个请求是要请求css还是html，所以转发到mako devServer后404了。

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1ad73b1 and b053b14.

Files selected for processing (1)

• scripts/test-e2e.mjs (4 hunks)

Files skipped from review as they are similar to previous changes (1)

• scripts/test-e2e.mjs

[coderabbitai]

Actionable comments posted: 0

Outside diff range and nitpick comments (1)

packages/bundler-mako/index.js (1)

121-130: 代码看起来不错，有一点小建议。

这段代码通过 express-http-proxy 中间件将请求直接代理到本地 HMR 服务器，并根据请求方法（GET 或 HEAD）进行过滤。对于代理响应状态码不是 200 的情况，会跳过当前处理程序。总体而言，这是一个合理的实现。

不过为了更好的可观测性，建议对代理响应状态码不是 200 的情况进行日志记录，例如：

  app.use(
    proxy(`http://127.0.0.1:${hmrPort}`, {
      filter: function (req, res) {
        return req.method == 'GET' || req.method == 'HEAD';
      },
      skipToNextHandlerFilter: function (proxyRes) {
+       if (proxyRes.statusCode !== 200) {
+         console.log(`Proxy response status code: ${proxyRes.statusCode}`);
+       }
        return proxyRes.statusCode !== 200;
      },
    }),
  );

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between dd9b152 and 9d6bdec.

Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

Files selected for processing (7)

• crates/mako/src/dev.rs (3 hunks)
• e2e/fixtures.umi/dev.hmr/.env (1 hunks)
• e2e/fixtures.umi/dev.hmr/expect.js (1 hunks)
• e2e/fixtures.umi/dev.hmr/global.css (1 hunks)
• packages/bundler-mako/index.js (2 hunks)
• packages/bundler-mako/package.json (1 hunks)
• scripts/test-e2e.mjs (2 hunks)

Files skipped from review due to trivial changes (2)

• e2e/fixtures.umi/dev.hmr/.env
• e2e/fixtures.umi/dev.hmr/global.css

Files skipped from review as they are similar to previous changes (3)

• crates/mako/src/dev.rs
• e2e/fixtures.umi/dev.hmr/expect.js
• scripts/test-e2e.mjs

Additional comments not posted (2)

packages/bundler-mako/package.json (1)

12-12: 添加依赖项 express-http-proxy 看起来不错！

引入 express-http-proxy 包可以增强应用程序通过 Express 处理 HTTP 代理功能的能力。这个包可以方便地将请求路由到不同的服务器或服务，从而提高应用程序管理 API 调用或充当代理请求中间件的能力。版本约束 ^2.1.1 允许进行次要版本和补丁更新，确保与应用程序的兼容性。

packages/bundler-mako/index.js (1)

85-85: 看起来不错！

引入 express-http-proxy 模块作为 proxy 变量，为后续的请求代理做准备。