Skip to content

Conversation

kalsolio
Copy link

PR 标题

简要描述这次 PR 的目的和内容

相关 Issue

关闭或关联的 Issue (如有):

变更类型

请勾选适用的变更类型:

  • Bug 修复 (不兼容变更的修复)
  • 新功能 (不兼容变更的新功能)
  • 功能改进 (不兼容现有功能的改进)
  • 文档更新
  • 依赖更新
  • 重构 (不影响功能的代码修改)
  • 测试用例
  • CI/CD 配置变更
  • 其他 (请描述):

变更内容

详细描述本次 PR 的具体变更内容:
1.
2.
3.

测试情况

描述本次变更的测试情况:

  • 已本地测试
  • 已添加测试用例
  • 不需要测试 (理由: )

其他说明

任何其他需要说明的事项:

@xiaomakuaiz
Copy link
Collaborator

@MonkeyCode-AI review

MonkeyCode-AI

This comment was marked as duplicate.

MonkeyCode-AI

This comment was marked as duplicate.

@yokowu
Copy link
Contributor

yokowu commented Sep 19, 2025

@MonkeyCode-AI review

Copy link
Collaborator

@MonkeyCode-AI MonkeyCode-AI left a comment

Choose a reason for hiding this comment

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

Pull Request 概述

本次 PR 仅包含一个文件的微小变更,将首页重定向逻辑从 NextResponse.redirect 改为 NextResponse.rewrite。从功能性和用户体验角度来看,这是一个合理的改进,因为 rewrite 方法可以保持 URL 不变的同时显示目标页面内容,提供更好的用户体验。

安全性: 无安全风险,变更仅涉及路由重定向方式
性能: 无性能影响,两种方法性能相当
可靠性: 变更逻辑简单明确,错误处理保持不变
功能性: 正确实现了需求,保持原有功能
可维护性: 代码结构清晰,注释明确
扩展性: 不影响系统扩展性
用户体验: 改进用户体验,URL 保持不变
兼容性: 完全向后兼容
合规性: 无合规性问题

Pull Request 变更详情

文件路径 变更类型 变更内容
web/app/src/middleware/home.ts 修改 将首页重定向从 redirect 改为 rewrite,保持 URL 不变

// }
if (url.pathname === '/') {
return NextResponse.redirect(new URL('/welcome', request.url));
// return NextResponse.redirect(new URL('/welcome', request.url));
Copy link
Collaborator

Choose a reason for hiding this comment

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

建议添加注释说明为什么使用 rewrite 而不是 redirect,以便其他开发者理解这个设计决策

Suggested change
// return NextResponse.redirect(new URL('/welcome', request.url));
// 使用 rewrite 而不是 redirect 以保持 URL 不变,提供更好的用户体验
return NextResponse.rewrite(new URL('/welcome', request.url));

@guanweiwang
Copy link
Contributor

@kalsolio 可能会存在问题,需要将页面上使用到 /welcome 路径需要更改,使用 rewrite 后,在页面上获取到的路径都是 '/' 而不是 '/welcome', 可能存在问题

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants