fix: dev server support "publicPath" #1398
Conversation
Walkthrough这些更改主要涉及在处理请求 URL 时引入了新的函数 Changes
Assessment against linked issues
Poem
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? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- crates/mako/src/dev/mod.rs (2 hunks)
- crates/mako/src/utils/mod.rs (1 hunks)
- packages/bundler-mako/index.js (1 hunks)
Additional comments not posted (2)
crates/mako/src/utils/mod.rs (1)
50-81: 测试用例: 通过测试用例覆盖了多种情况，验证了
process_req_url函数的行为。测试用例全面且正确。packages/bundler-mako/index.js (1)
127-132: 中间件: 通过中间件正确地使用了
processReqURL来调整基于公共路径配置的请求URL。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- crates/mako/src/dev/mod.rs (2 hunks)
- crates/mako/src/utils/mod.rs (1 hunks)
Additional comments not posted (4)
crates/mako/src/utils/mod.rs (2)
36-42: 功能实现良好！
process_req_url函数正确地处理了基于public_path的请求 URL，并且考虑了各种情况。
44-79: 单元测试覆盖全面！单元测试涵盖了各种场景，确保了
process_req_url函数的可靠性。crates/mako/src/dev/mod.rs (2)
127-139: 功能集成良好！在
DevServer::handle_requests中正确地集成了process_req_url函数，并且适当地处理了错误。
130-138: 错误处理适当！在
process_req_url失败时，错误处理返回BAD_REQUEST响应，确保了无效路径的正确管理。
whyer11
force-pushed
the
fix/devserver_support_publicpath
branch
from
8630d60
to
b6dd925
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- crates/mako/src/dev/mod.rs (2 hunks)
- crates/mako/src/utils/mod.rs (1 hunks)
- packages/bundler-mako/index.js (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- crates/mako/src/dev/mod.rs
- crates/mako/src/utils/mod.rs
- packages/bundler-mako/index.js
…icPath" configuration (#1334)
whyer11
force-pushed
the
fix/devserver_support_publicpath
branch
from
b6dd925
to
da6d209
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- crates/mako/src/dev/mod.rs (2 hunks)
- crates/mako/src/utils/mod.rs (1 hunks)
- packages/bundler-mako/index.js (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- crates/mako/src/utils/mod.rs
- packages/bundler-mako/index.js
Additional comments not posted (1)
crates/mako/src/dev/mod.rs (1)
127-139: 建议改进: 增加错误处理在调用
process_req_url时，建议增加错误处理，以防止潜在的路径处理错误导致服务器崩溃。let path = match process_req_url(public_path, &path) { Ok(p) => p, Err(e) => { eprintln!("Error processing request URL: {:?}", e); return Ok(hyper::Response::builder() .status(hyper::StatusCode::BAD_REQUEST) .body(hyper::Body::from("Bad Request")) .unwrap()); } };
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
和 @stormslowly 讨论了下，按这个逻辑来。至于 writeToDisk 的场景，后续增加 compiling 状态来决定是否 pending request 来解决。 |
packages/bundler-mako/index.js
Outdated
| function processReqURL(publicPath, reqURL) { | ||
| if (!publicPath.startsWith('/')) { | ||
| publicPath = '/' + publicPath; | ||
| } | ||
| if (reqURL.startsWith(publicPath)) { | ||
| return reqURL.slice(publicPath.length - 1); | ||
| } else { | ||
| return reqURL; | ||
| } | ||
| } |
There was a problem hiding this comment.
建议改进: 优化 processReqURL 函数
目前的逻辑可以进一步简化和优化，以提高代码的可读性和处理边界情况的能力。
function processReqURL(publicPath, reqURL) {
if (!publicPath.startsWith('/')) {
publicPath = '/' + publicPath;
}
publicPath = publicPath.replace(/\/+$/, '') + '/';
if (reqURL.startsWith(publicPath)) {
return reqURL.slice(publicPath.length - 1);
}
return reqURL;
}这样可以确保 publicPath 总是以一个斜杠开头和结尾，并且处理了多个结尾斜杠的情况。
packages/bundler-mako/index.js
Outdated
| if (opts.config.publicPath) { | ||
| app.use((req, res, next) => { | ||
| req.url = processReqURL(opts.config.publicPath, req.url); | ||
| next(); | ||
| }); |
There was a problem hiding this comment.
建议改进: 提高中间件的可读性
目前的中间件逻辑可以通过解构 opts.config 来提高可读性。
const { publicPath } = opts.config;
if (publicPath) {
app.use((req, res, next) => {
req.url = processReqURL(publicPath, req.url);
next();
});
}这样可以使代码更加简洁和易读。
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- crates/mako/src/dev/mod.rs (2 hunks)
- crates/mako/src/utils/mod.rs (1 hunks)
Additional comments not posted (1)
crates/mako/src/utils/mod.rs (1)
44-62: 测试模块通过!测试模块覆盖了不同的情况，确保函数按预期运行。测试编写良好且全面。
| pub fn process_req_url(public_path: &str, req_url: &str) -> Result<String> { | ||
| let public_path = format!("/{}/", public_path.trim_matches('/')); | ||
| if req_url.starts_with(&public_path) { | ||
| return Ok(req_url[public_path.len() - 1..].to_string()); | ||
| } | ||
| Ok(req_url.to_string()) |
There was a problem hiding this comment.
建议改进: 优化字符串处理逻辑
目前的逻辑虽然正确，但可以使用标准库提供的方法来简化字符串的处理。例如，可以使用 strip_prefix 和 strip_suffix 方法来处理路径。
pub fn process_req_url(public_path: &str, req_url: &str) -> Result<String> {
let public_path = format!("/{}/", public_path.trim_matches('/'));
if req_url.starts_with(&public_path) {
return Ok(req_url[public_path.len() - 1..].to_string());
}
Ok(req_url.to_string())
}| let mut path = req.uri().path().to_string(); | ||
| let public_path = &context.config.public_path; | ||
| if !public_path.is_empty() && public_path.starts_with('/') && public_path != "/" { | ||
| path = match process_req_url(public_path, &path) { | ||
| Ok(p) => p, | ||
| Err(_) => { | ||
| return Ok(hyper::Response::builder() | ||
| .status(hyper::StatusCode::BAD_REQUEST) | ||
| .body(hyper::Body::from("Bad Request")) | ||
| .unwrap()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
建议改进: 增加错误处理
在调用 process_req_url 时，建议增加错误处理，以防止潜在的路径处理错误导致服务器崩溃。
let path = match process_req_url(public_path, &path) {
Ok(p) => p,
Err(e) => {
eprintln!("Error processing request URL: {:?}", e);
return Ok(hyper::Response::builder()
.status(hyper::StatusCode::BAD_REQUEST)
.body(hyper::Body::from("Bad Request"))
.unwrap());
}
};
Fixed the issue where the development server was unable to correctly consume the publicPath configuration.
Close #1424
Close #1153
Close #1334
Summary by CodeRabbit
Summary by CodeRabbit
BAD_REQUEST状态反馈。