-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: devserver response header add cacheControl no-cache #1692
Conversation
Walkthrough此拉取请求对 Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request addresses an issue reported by Ant Design where refreshing the browser would fetch outdated resources. To ensure proper development, the browser cache is disabled by adding a Changes
|
@@ -179,6 +179,7 @@ impl DevServer { | |||
|
|||
return Ok(hyper::Response::builder() | |||
.status(hyper::StatusCode::OK) | |||
.header(CACHE_CONTROL, "no-cache") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that adding the Cache-Control: no-cache
header does not interfere with other caching mechanisms or headers that might be required for different environments or use cases. Consider making this configurable if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/dev.rs (1)
182-182
: 缓存控制头部添加正确,建议扩展应用范围!当前实现正确地在静态内容响应中添加了
no-cache
缓存控制,这有效解决了开发过程中的资源缓存问题。不过建议也为缓存的依赖响应添加相同的缓存控制头部,以确保一致的缓存行为。
建议在处理缓存依赖的响应时也添加相同的头部:
return std::fs::read(abs_path).map_or(Ok(not_found_response()), |bytes| { Ok(hyper::Response::builder() .status(hyper::StatusCode::OK) + .header(CACHE_CONTROL, "no-cache") .header(CONTENT_TYPE, content_type) .body(hyper::Body::from(bytes)) .unwrap()) });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
crates/mako/src/dev.rs
(2 hunks)
🔇 Additional comments (1)
crates/mako/src/dev.rs (1)
13-13
: 导入声明修改正确!
引入 CACHE_CONTROL
头部常量的方式符合 Rust 的导入语法规范,并且与现有的 CONTENT_TYPE
导入保持一致。
@Jinbao1001 是不是也把跨域的头给加上 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1692 +/- ##
=======================================
Coverage 55.28% 55.28%
=======================================
Files 175 175
Lines 17696 17696
=======================================
Hits 9783 9783
Misses 7913 7913 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Ant Design reported that refreshing the browser would fetch old resources, and disabling browser cache is mandatory for normal development. Therefore, we force the browser to disable cache.
Summary by CodeRabbit
CACHE_CONTROL: no-cache
和ACCESS_CONTROL_ALLOW_ORIGIN: *
头部,优化了客户端缓存行为和跨域请求处理。