feat: output filename #1725
feat: output filename #1725
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 演练这个拉取请求引入了对 Mako 构建系统的配置和文件生成逻辑的一系列增强。主要变更包括在输出配置中添加可选的文件名字段，修改 Chunk 命名方法，并在生成过程中支持自定义文件名模板。这些更改提供了更灵活的文件输出和命名机制，同时保持了现有系统的核心功能。 变更
可能相关的 PRs
建议的审阅者
诗歌
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 introduces a new feature that allows specifying an output filename in the configuration. It modifies several components to accommodate this change, including the addition of a Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1725 +/- ##
==========================================
+ Coverage 54.77% 54.83% +0.05%
==========================================
Files 181 181
Lines 17981 18021 +40
==========================================
+ Hits 9849 9881 +32
- Misses 8132 8140 +8 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
crates/mako/src/generate/chunk.rs (1)
Line range hint
76-115: 建议改进错误处理和命名规范
name方法中存在以下问题：
parse_path的错误处理使用了unwrap()，建议使用Result处理错误- 文件名中的特殊字符替换可以抽取为单独的函数
- 命名规则的常量应该提取出来
建议如下重构：
const SPECIAL_CHARS: [char; 3] = ['.', '?', '@']; const SPECIAL_SEGMENTS: &[(&str, &str)] = &[ ("pd_", ".."), ("ps_", ""), ]; impl Chunk { fn sanitize_path_segment(segment: &str) -> String { segment.replace(SPECIAL_CHARS, "_") } pub fn name(&self) -> Result<String, ChunkError> { match &self.chunk_type { ChunkType::Runtime => Ok("runtime".into()), ChunkType::Entry(_, name, _) => Ok(name.clone()), ChunkType::Async | ChunkType::Sync | ChunkType::Worker(_) => { let (path, search, ..) = parse_path(&self.id.id) .map_err(|e| ChunkError::PathParsing(e))?; // ... rest of the implementation } } } }
🧹 Nitpick comments (5)
e2e/fixtures/config.output.filename/expect.js (1)
8-10: 建议优化测试用例的健壮性当前测试仅验证了文件名的基本格式，建议增加以下测试场景：
- 验证不同的 name 值
- 验证 hash 的长度和格式
- 验证生成的文件内容是否正确
const {files} = parseBuildResult(__dirname); let filename = Object.keys(files)[0] - -expect(filename).toMatch(/index\.umd\..+\.js/) + +describe('output filename', () => { + it('should match the expected pattern', () => { + expect(filename).toMatch(/index\.umd\..+\.js/); + expect(filename.split('.')[2]).toMatch(/^[a-f0-9]{8}$/); + }); + + it('should contain valid file content', () => { + expect(files[filename]).toBeTruthy(); + }); +});crates/mako/src/generate/chunk_pot.rs (1)
29-29: 建议为新字段添加文档注释新增的
chunk_name字段看起来没有问题，但建议添加文档注释说明其用途和与filename选项的关系。添加如下文档注释:
+ /// 代表块的名称，用于生成输出文件名 + /// 当启用 filename 选项时会被用作模板变量 pub chunk_name: String,crates/mako/src/generate/generate_chunks.rs (3)
36-40: 结构体字段需要添加文档注释新增的字段
chunk_name和file_name_template实现良好，但建议为这些字段添加文档注释以提高代码可维护性。建议添加如下文档注释：
#[derive(Clone)] pub struct ChunkFile { pub raw_hash: u64, pub content: Vec<u8>, pub source_map: Option<Vec<u8>>, pub hash: Option<String>, + /// 块的名称，用于文件名模板中的 [name] 占位符 pub chunk_name: String, pub file_name: String, pub chunk_id: String, pub file_type: ChunkFileType, + /// 可选的文件名模板，支持 [name]、[id]、[hash] 和 [contenthash] 占位符 pub file_name_template: Option<String>, }
301-342: 建议重构复杂的占位符替换逻辑当前函数职责过重，嵌套层级较深，建议拆分为更小的函数以提高可维护性。
建议将内部逻辑拆分为独立函数：
fn find_and_replace_placeholder( content: &mut Vec<u8>, placeholder: &str, replacer: &str, ) -> Result<()> { let position = content .windows(placeholder.len()) .position(|w| w == placeholder.as_bytes()) .ok_or_else(|| anyhow!("Placeholder '{}' not found", placeholder))?; content.splice( position..position + replacer.len(), replacer.as_bytes().to_vec(), ); Ok(()) } fn replace_chunks_placeholder( chunk_files: &mut [ChunkFile], chunks_hash_placeholder: &ChunksHashPlaceholder, chunks_hash_replacer: &ChunksHashReplacer, ) -> Result<()> { for (chunk_id, placeholder) in chunks_hash_placeholder { let replacer = chunks_hash_replacer .get(chunk_id) .ok_or_else(|| anyhow!( "Replacer not found for chunk '{}' with placeholder '{}'", chunk_id, placeholder ))?; for cf in chunk_files .iter_mut() .filter(|cf| matches!(cf.file_type, ChunkFileType::JS)) { if cf.content.is_empty() { warn!("Chunk content of \"{}\" is empty.", cf.chunk_id); continue; } find_and_replace_placeholder(&mut cf.content, placeholder, replacer)?; } } Ok(()) }
438-458: 建议增加更多测试用例当前测试仅覆盖了基本场景，建议添加以下测试场景：
- 无模板时的默认行为
- 模板中包含无效占位符
- 不同文件类型的处理
- hash 为 None 的情况
#[test] fn test_template_edge_cases() { let base_chunk_file = ChunkFile { raw_hash: 0, content: vec![], source_map: None, hash: Some("hash999".to_string()), chunk_name: "chunk".to_string(), file_name: "index.js".to_string(), chunk_id: "c_id".to_string(), file_type: ChunkFileType::JS, file_name_template: None, }; // 测试无模板时的默认行为 assert_eq!(base_chunk_file.disk_name(), "index.js"); // 测试无效占位符 let mut cf = base_chunk_file.clone(); cf.file_name_template = Some("[invalid].[name].js".to_string()); assert_eq!(cf.disk_name(), "[invalid].chunk.js"); // 测试 hash 为 None 的情况 let mut cf = base_chunk_file.clone(); cf.hash = None; cf.file_name_template = Some("[name].[hash].js".to_string()); assert_eq!(cf.disk_name(), "chunk.notHashed.js"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
crates/mako/src/config/output.rs(1 hunks)crates/mako/src/generate/chunk.rs(3 hunks)crates/mako/src/generate/chunk_pot.rs(2 hunks)crates/mako/src/generate/chunk_pot/ast_impl.rs(4 hunks)crates/mako/src/generate/chunk_pot/str_impl.rs(2 hunks)crates/mako/src/generate/generate_chunks.rs(5 hunks)e2e/fixtures/config.output.filename/expect.js(1 hunks)e2e/fixtures/config.output.filename/mako.config.json(1 hunks)e2e/fixtures/config.output.filename/src/index.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- e2e/fixtures/config.output.filename/src/index.tsx
- e2e/fixtures/config.output.filename/mako.config.json
🔇 Additional comments (9)
crates/mako/src/generate/chunk.rs (1)
120-122:
建议处理 name() 方法可能的错误
filename 方法直接使用了 name() 的结果，没有处理潜在的错误。
pub fn filename(&self) -> String {
- format!("{}.js", self.name())
+ format!("{}.js", self.name().unwrap_or_else(|e| {
+ log::error!("Failed to generate chunk name: {}", e);
+ "unknown".to_string()
+ }))
}Likely invalid or redundant comment.
crates/mako/src/generate/chunk_pot.rs (1)
45-45: 字段初始化逻辑正确
在 from 方法中使用 chunk.name() 初始化 chunk_name 字段的实现是正确的。
crates/mako/src/generate/chunk_pot/ast_impl.rs (4)
136-137: CSS 块文件添加了必要的文件名支持
render_css_chunk 函数中正确添加了 chunk_name 和 file_name_template 字段。将 file_name_template 设置为 None 是合理的，因为 CSS 块不需要自定义文件名模板。
186-186: JS 块文件添加了必要的文件名支持
render_normal_js_chunk 函数中正确添加了 chunk_name 和 file_name_template 字段。将 file_name_template 设置为 None 是合理的，因为普通 JS 块不需要自定义文件名模板。
Also applies to: 189-189
229-230: 入口块文件正确使用配置的文件名模板
render_entry_js_chunk 函数中正确设置了 chunk_name 和 file_name_template，并且适当地使用了配置中的 filename 选项。
333-333: 请验证哈希计算的更改
移除了条件检查直接计算哈希的改动可能会影响性能，建议验证此更改的必要性和影响。
crates/mako/src/generate/chunk_pot/str_impl.rs (2)
106-107: 入口块的字符串实现正确支持文件名选项
render_entry_js_chunk 的字符串实现中正确添加了文件名相关字段，与 AST 实现保持一致。
157-158: 普通块的字符串实现正确支持文件名选项
render_normal_js_chunk 的字符串实现中正确添加了文件名相关字段，与 AST 实现保持一致。
crates/mako/src/generate/generate_chunks.rs (1)
44-48: 磁盘文件名生成逻辑合理
方法实现简洁明了，通过 Option 优雅地处理了模板存在与否的情况。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/mako/src/generate/generate_chunks.rs (2)
301-342: 建议重构以提高代码可读性！当前的
replace_chunks_placeholder函数包含了较多的嵌套操作和复杂的错误处理逻辑。建议将其拆分为更小的函数以提高可维护性。建议将内部逻辑抽取为独立函数：
+ fn replace_single_chunk_placeholder( + chunk_files: &mut [ChunkFile], + chunk_id: &str, + placeholder: &str, + replacer: &str, + ) -> Result<()> { + chunk_files + .iter_mut() + .filter(|cf| matches!(cf.file_type, ChunkFileType::JS)) + .try_for_each(|cf| { + if cf.content.is_empty() { + warn!("Chunk content of \"{}\" is empty.", cf.chunk_id); + } + + let position = cf + .content + .windows(placeholder.len()) + .position(|w| w == placeholder.as_bytes()); + + position.map_or( + Err(anyhow!( + "Generate \"{}\" failed, placeholder \"{}\" for \"{}\" not existed in chunk file.", + cf.chunk_id, + placeholder, + chunk_id + )), + |pos| { + cf.content.splice( + pos..pos + replacer.len(), + replacer.as_bytes().to_vec(), + ); + Ok(()) + }, + ) + }) + } fn replace_chunks_placeholder( chunk_files: &mut [ChunkFile], chunks_hash_placeholder: &ChunksHashPlaceholder, chunks_hash_replacer: &ChunksHashReplacer, ) -> Result<()> { chunks_hash_placeholder.iter().try_for_each(|(chunk_id, placeholder)| { chunks_hash_replacer .get(chunk_id) .map_or( Err(anyhow!( "Generate \"{}\" failed, replacer not found for placeholder \"{}\".", chunk_id, placeholder )), |replacer| replace_single_chunk_placeholder(chunk_files, chunk_id, placeholder, replacer), ) }) }
438-458: 建议增加更多测试用例！当前的测试虽然覆盖了基本功能，但建议添加以下测试场景：
- 空模板的处理
- 无效占位符的处理
- 缺少 hash 值时的处理
- 特殊字符在文件名中的处理
建议添加如下测试用例：
#[test] fn test_template_edge_cases() { let base_chunk_file = ChunkFile { raw_hash: 0, content: vec![], source_map: None, hash: Some("hash999".to_string()), chunk_name: "chunk".to_string(), file_name: "index.js".to_string(), chunk_id: "c_id".to_string(), file_type: ChunkFileType::JS, file_name_template: None, }; // 测试空模板 let mut chunk_file = base_chunk_file.clone(); chunk_file.file_name_template = Some("".to_string()); assert_eq!(chunk_file.disk_name(), ""); // 测试无效占位符 let mut chunk_file = base_chunk_file.clone(); chunk_file.file_name_template = Some("[invalid].js".to_string()); assert_eq!(chunk_file.disk_name(), "[invalid].js"); // 测试缺少 hash 值 let mut chunk_file = base_chunk_file.clone(); chunk_file.hash = None; chunk_file.file_name_template = Some("[name].[hash].js".to_string()); assert_eq!(chunk_file.disk_name(), "chunk.notHashed.js"); // 测试特殊字符 let mut chunk_file = base_chunk_file; chunk_file.chunk_name = "special@#$".to_string(); chunk_file.file_name_template = Some("[name].[hash].js".to_string()); assert_eq!(chunk_file.disk_name(), "special@#$.hash999.js"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/mako/src/generate/generate_chunks.rs(5 hunks)
🔇 Additional comments (3)
crates/mako/src/generate/generate_chunks.rs (3)
36-40: 结构体字段添加合理！
新增的 chunk_name 和 file_name_template 字段设计合理，类型选择恰当。chunk_name 作为必填字段而 file_name_template 作为可选字段的设计符合实际使用场景。
45-48: 方法重构清晰简洁！
disk_name 方法的重构采用了优雅的模板优先策略，当没有模板时回退到默认实现。这种设计既保持了向后兼容性，又提供了新功能的扩展。
70-78: 🛠️ Refactor suggestion
建议增加模板验证和错误处理！
当前的模板渲染实现虽然简单直接，但建议增加以下改进：
- 添加模板格式的验证
- 对无效模板提供友好的错误提示
建议按如下方式改进：
fn render_tmpl(&self, tpl: &str) -> String {
let hash_string = self.hash.as_deref().unwrap_or("notHashed");
+ // 验证模板中是否包含有效的占位符
+ if !tpl.contains("[name]") && !tpl.contains("[id]") &&
+ !tpl.contains("[hash]") && !tpl.contains("[contenthash]") &&
+ !tpl.contains("[file]") {
+ warn!("模板 '{}' 中未包含任何有效的占位符", tpl);
+ }
+
tpl.replace("[name]", self.chunk_name.as_str())
.replace("[id]", self.chunk_id.as_str())
.replace("[file]", self.file_name.as_str())
.replace("[hash]", hash_string)
.replace("[contenthash]", hash_string)
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
codecov.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
codecov.yml
[error] 6-6: syntax error: could not find expected ':'
(syntax)
codecov.yml
Outdated
| target: auto | ||
| thre |
There was a problem hiding this comment.
修复 YAML 语法错误和不完整的配置
thre 配置项似乎不完整，这会导致 YAML 解析错误。这可能是想要配置 threshold（阈值）。
建议按照以下方式修改：
project:
target: auto
- thre
+ threshold: null # 或设置具体的阈值，例如 80📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| target: auto | |
| thre | |
| target: auto | |
| threshold: null # 或设置具体的阈值，例如 80 |
stormslowly
force-pushed
the
feat/output_filename
branch
from
cd3654b to
faf3f67
Compare
stormslowly
force-pushed
the
feat/output_filename
branch
from
faf3f67 to
0d3594f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
codecov.yml (1)
4-4: 建议添加文件末尾的换行符根据 YAML 规范和最佳实践，文件应以换行符结束。
coverage: status: project: off - patch: off + patch: off +🧰 Tools
🪛 yamllint (1.35.1)
[error] 4-4: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
codecov.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
codecov.yml
[error] 4-4: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
codecov.yml (1)
1-4: 请确认是否有意禁用所有覆盖率报告
当前配置完全禁用了项目和补丁的覆盖率报告。这可能会影响代码质量监控。建议仔细考虑以下替代方案：
coverage:
status:
- project: off
- patch: off
+ project:
+ default:
+ target: auto
+ threshold: 1%
+ patch:
+ default:
+ target: 80%✅ Verification successful
建议保持当前的覆盖率配置禁用状态
根据代码库分析，当前禁用覆盖率报告是合理的：
- 项目主要使用 Rust 开发，已经在
Code Coverage工作流中通过grcov和codecov-action单独配置了 Rust 代码的覆盖率报告。 - JavaScript/TypeScript 测试较少，主要是端到端测试（E2E）和热更新（HMR）测试，这类测试不适合使用代码覆盖率指标。
- 现有的测试策略更注重功能验证而非代码覆盖率。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查仓库中的测试文件，确认是否确实不需要覆盖率报告
# 搜索测试文件
echo "查找测试文件..."
fd -e test.ts -e test.js -e spec.ts -e spec.js
# 检查是否有 CI 配置使用覆盖率
echo "检查 CI 配置..."
fd -t f ".github/workflows" -x cat {}
Length of output: 622
Script:
#!/bin/bash
# 检查测试文件内容和 CI 配置
echo "检查测试文件内容..."
cat packages/rsc/src/index.test.ts
echo -e "\n检查 CI 工作流配置..."
fd . '.github/workflows' --full-path --type f --exec cat {}
echo -e "\n检查 package.json 中的测试脚本..."
fd package.json --type f --exec cat {} | grep -A 5 -B 5 '"test":'
Length of output: 18455
🧰 Tools
🪛 yamllint (1.35.1)
[error] 4-4: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/bundler-mako/package.json(1 hunks)packages/mako/npm/darwin-arm64/package.json(1 hunks)packages/mako/npm/darwin-x64/package.json(1 hunks)packages/mako/npm/linux-arm64-gnu/package.json(1 hunks)packages/mako/npm/linux-arm64-musl/package.json(1 hunks)packages/mako/npm/linux-x64-gnu/package.json(1 hunks)packages/mako/npm/linux-x64-musl/package.json(1 hunks)packages/mako/npm/win32-ia32-msvc/package.json(1 hunks)packages/mako/npm/win32-x64-msvc/package.json(1 hunks)packages/mako/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (9)
- packages/mako/npm/darwin-arm64/package.json
- packages/mako/npm/linux-arm64-musl/package.json
- packages/mako/npm/darwin-x64/package.json
- packages/mako/npm/win32-ia32-msvc/package.json
- packages/mako/npm/linux-x64-musl/package.json
- packages/bundler-mako/package.json
- packages/mako/npm/win32-x64-msvc/package.json
- packages/mako/npm/linux-x64-gnu/package.json
- packages/mako/npm/linux-arm64-gnu/package.json
🔇 Additional comments (1)
packages/mako/package.json (1)
3-3: 版本号更新符合语义化版本规范
版本号从 0.9.9 更新到 0.9.10-canary.20241218.1 合理反映了新功能的添加（filename 输出支持）。使用 canary 标签进行预发布也是一个谨慎的做法。
| "@umijs/mako-darwin-arm64": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-linux-arm64-gnu": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-linux-arm64-musl": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-win32-ia32-msvc": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-darwin-x64": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-win32-x64-msvc": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-linux-x64-gnu": "0.9.10-canary.20241218.1", | ||
| "@umijs/mako-linux-x64-musl": "0.9.10-canary.20241218.1" |
There was a problem hiding this comment.
💡 Codebase verification
版本一致性验证需要注意
从验证结果来看，虽然 packages/mako/package.json 中的可选依赖声明都使用了相同的版本 0.9.10-canary.20241218.1，但实际的 npm 包中存在一些不一致：
win32-arm64-msvc、linux-arm-gnueabihf和darwin-universal包的实际版本仍然是0.0.0- 其他平台的包版本已经正确更新到
0.9.10-canary.20241218.1
建议：
- 确保所有平台的 npm 包都已经正确发布并更新到最新版本
- 特别关注
win32-arm64-msvc、linux-arm-gnueabihf和darwin-universal这几个平台的包是否需要更新
🔗 Analysis chain
可选依赖版本保持一致性
所有平台特定的可选依赖包版本都已同步更新到 0.9.10-canary.20241218.1，保持了版本的一致性。
运行以下脚本验证所有相关包的版本一致性：
Let's analyze the output more carefully. I can see that there are some platform-specific packages that have version "0.0.0", which is inconsistent with the versions shown in package.json dependencies. Let me verify this further.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 验证所有 @umijs/mako 相关包的版本一致性
# 检查所有相关包的 package.json 中的版本号
fd -t f "package.json" -x grep -H "\"version\":" {} \;
Length of output: 3789
Script:
#!/bin/bash
# Check the actual content of package.json to verify the dependencies
cat ./packages/mako/package.json | grep -A 10 "optionalDependencies"
Length of output: 641
a simple support for
filenamein output configSummary by CodeRabbit
新功能
OutputConfig结构中添加了可选字段filename。Chunk结构中添加了新的filename方法，并重命名了现有的filename方法为name。ChunkPot结构中添加了新的chunk_name字段。ChunkFile结构中添加了chunk_name和file_name_template字段。mako.config.json，定义了动态生成的输出文件名模板。expect.js，用于验证构建过程中的输出文件名。样式
src/index.tsx中添加了简单的日志输出功能。其他
codecov.yml中添加了新的覆盖率配置。package.json文件中的版本号，统一从0.9.9更新至0.9.10-canary.20241218.1。