-
Notifications
You must be signed in to change notification settings - Fork 79
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
feat: add regexes from node_modules watch #1439
Conversation
Walkthrough此次更改增强了 Changes
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 (
|
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: 3
Outside diff range, codebase verification and nitpick comments (3)
crates/mako/src/dev/watch.rs (1)
61-61
: 建议:检查配置项node_modules_regexes
是否存在在使用
node_modules_regexes
之前,建议检查配置项是否存在,以避免潜在的None
值错误。+ let node_modules_regexes = &self.compiler.context.config.watch.node_modules_regexes.unwrap_or(&vec![]);
crates/mako/src/config/config.rs (2)
449-449
: 建议:为新字段node_modules_regexes
添加文档注释为了提高代码可读性和可维护性,建议为新字段
node_modules_regexes
添加文档注释。/// 正则表达式列表,用于过滤 `node_modules` 目录中的文件 pub node_modules_regexes: Vec<String>,
723-723
: 建议:为新字段nodeModulesRegexes
添加默认值为了确保配置的一致性,建议为新字段
nodeModulesRegexes
添加一个默认值。- "nodeModulesRegexes": [] + "nodeModulesRegexes": [],
crates/mako/src/dev/watch.rs
Outdated
if !node_modules_regexes.is_empty() { | ||
let regexes = node_modules_regexes.iter().map(|s| Regex::new(s).unwrap()); |
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.
建议:处理正则表达式编译错误
当前代码假设正则表达式总是有效的。如果正则表达式无效,会导致 unwrap
引发恐慌。建议处理正则表达式编译错误。
- let regexes = node_modules_regexes.iter().map(|s| Regex::new(s).unwrap());
+ let regexes = node_modules_regexes.iter().map(|s| Regex::new(s)).collect::<Result<Vec<_>, _>>()?;
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.
if !node_modules_regexes.is_empty() { | |
let regexes = node_modules_regexes.iter().map(|s| Regex::new(s).unwrap()); | |
if !node_modules_regexes.is_empty() { | |
let regexes = node_modules_regexes.iter().map(|s| Regex::new(s)).collect::<Result<Vec<_>, _>>()?; |
crates/mako/src/dev/watch.rs
Outdated
let is_match = regexes | ||
.into_iter() | ||
.any(|regex| regex.is_match(resource.0.path().to_str().unwrap())); |
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.
建议:优化正则表达式匹配逻辑
将正则表达式编译和匹配逻辑分开,以提高代码可读性和维护性。
- let is_match = regexes.into_iter().any(|regex| regex.is_match(resource.0.path().to_str().unwrap()));
+ let is_match = regexes.iter().any(|regex| regex.is_match(resource.0.path().to_str().unwrap()));
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.
let is_match = regexes | |
.into_iter() | |
.any(|regex| regex.is_match(resource.0.path().to_str().unwrap())); | |
let is_match = regexes | |
.iter() | |
.any(|regex| regex.is_match(resource.0.path().to_str().unwrap())); |
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/dev/watch.rs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/dev/watch.rs
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.
1、补充下背景?
2、介绍下 node_modules_regexes 的作用?可能得加个前缀,现在评名字不太能理解他的作用
3、配置需要同步调整 bindings/src/lib.rs 里的类型
4、配置需要同步更新 config.md 和 config.zh-CN.md
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- crates/binding/src/lib.rs (1 hunks)
- crates/mako/src/config/config.rs (2 hunks)
- crates/mako/src/dev/watch.rs (5 hunks)
- packages/mako/binding.d.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/config/config.rs
Additional comments not posted (3)
packages/mako/binding.d.ts (1)
186-186
: 新增属性nodeModulesRegexes
看起来很好。这个更改允许用户通过正则表达式来匹配
node_modules
目录中的路径,从而提高监控文件变化的灵活性和性能。crates/binding/src/lib.rs (1)
171-171
: 新增字段nodeModulesRegexes
看起来很好。这个更改允许用户通过正则表达式来匹配
node_modules
目录中的路径,从而提高监控文件变化的灵活性和性能。crates/mako/src/dev/watch.rs (1)
72-83
: 建议:优化正则表达式匹配逻辑将正则表达式编译和匹配逻辑分开,以提高代码可读性和维护性。
- let is_match = regexes.into_iter().any(|regex| regex.is_match(resource.0.path().to_str().unwrap())); + let is_match = regexes.iter().any(|regex| regex.is_match(resource.0.path().to_str().unwrap()));Likely invalid or redundant comment.
if is_match { | ||
let _ = self.watcher.watch( | ||
resource.0.path().to_path_buf().as_path(), | ||
notify::RecursiveMode::NonRecursive, | ||
); | ||
} |
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.
建议:处理 watch
方法的错误
当前代码忽略了 watch
方法的错误。建议处理这些错误,以便在发生错误时采取适当的行动。
- let _ = self.watcher.watch(resource.0.path().to_path_buf().as_path(), notify::RecursiveMode::NonRecursive);
+ if let Err(e) = self.watcher.watch(resource.0.path().to_path_buf().as_path(), notify::RecursiveMode::NonRecursive) {
+ eprintln!("Failed to watch path: {:?}", e);
+ }
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.
if is_match { | |
let _ = self.watcher.watch( | |
resource.0.path().to_path_buf().as_path(), | |
notify::RecursiveMode::NonRecursive, | |
); | |
} | |
if is_match { | |
if let Err(e) = self.watcher.watch( | |
resource.0.path().to_path_buf().as_path(), | |
notify::RecursiveMode::NonRecursive, | |
) { | |
eprintln!("Failed to watch path: {:?}", e); | |
} | |
} |
@@ -18,6 +19,7 @@ | |||
pub compiler: &'a Compiler, | |||
pub watched_files: HashSet<PathBuf>, | |||
pub watched_dirs: HashSet<PathBuf>, | |||
node_modules_regexes: Vec<Regex>, |
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.
建议:处理正则表达式编译错误
当前代码假设正则表达式总是有效的。如果正则表达式无效,会导致 unwrap
引发恐慌。建议处理正则表达式编译错误。
- let regexes = node_modules_regexes.iter().map(|s| Regex::new(s).unwrap());
+ let regexes = node_modules_regexes.iter().map(|s| Regex::new(s)).collect::<Result<Vec<_>, _>>()?;
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.
node_modules_regexes: Vec<Regex>, | |
node_modules_regexes: Vec<Regex>, | |
``` | |
```diff | |
- let regexes = node_modules_regexes.iter().map(|s| Regex::new(s).unwrap()); | |
+ let regexes = node_modules_regexes.iter().map(|s| Regex::new(s)).collect::<Result<Vec<_>, _>>()?; |
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/dev/watch.rs (5 hunks)
Additional comments not posted (2)
crates/mako/src/dev/watch.rs (2)
10-10
: 导入regex
crate导入
regex
crate 是为了支持正则表达式功能,这是必要的。
22-22
: 添加新字段node_modules_regexes
添加
node_modules_regexes
字段是为了存储编译后的正则表达式模式,这是必要的。
node_modules_regexes: compiler | ||
.context | ||
.config | ||
.watch | ||
.node_modules_regexes | ||
.clone() | ||
.unwrap_or_default() | ||
.iter() | ||
.map(|s| Regex::new(s).unwrap()) | ||
.collect::<Vec<Regex>>(), |
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.
建议:处理正则表达式编译错误
当前代码假设正则表达式总是有效的。如果正则表达式无效,会导致 unwrap
引发恐慌。建议处理正则表达式编译错误。
- let regexes = node_modules_regexes.iter().map(|s| Regex::new(s).unwrap());
+ let regexes = node_modules_regexes.iter().map(|s| Regex::new(s)).collect::<Result<Vec<_>, _>>()?;
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.
node_modules_regexes: compiler | |
.context | |
.config | |
.watch | |
.node_modules_regexes | |
.clone() | |
.unwrap_or_default() | |
.iter() | |
.map(|s| Regex::new(s).unwrap()) | |
.collect::<Vec<Regex>>(), | |
node_modules_regexes: compiler | |
.context | |
.config | |
.watch | |
.node_modules_regexes | |
.clone() | |
.unwrap_or_default() | |
- .iter() | |
- .map(|s| Regex::new(s).unwrap()) | |
- .collect::<Vec<Regex>>(), | |
+ .iter() | |
+ .map(|s| Regex::new(s)) | |
+ .collect::<Result<Vec<_>, _>>()?, |
if !self.node_modules_regexes.is_empty() { | ||
let file_path = resource.0.path().to_str().unwrap(); | ||
let is_match = file_path.contains("node_modules") | ||
&& self | ||
.node_modules_regexes | ||
.iter() | ||
.any(|regex| regex.is_match(file_path)); | ||
if is_match { | ||
let _ = self.watcher.watch( | ||
resource.0.path().to_path_buf().as_path(), | ||
notify::RecursiveMode::NonRecursive, | ||
); | ||
} | ||
} |
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.
优化正则表达式匹配逻辑和处理 watch
方法的错误
将正则表达式编译和匹配逻辑分开,以提高代码可读性和维护性。同时,当前代码忽略了 watch
方法的错误,建议处理这些错误。
- let is_match = file_path.contains("node_modules")
- && self.node_modules_regexes.iter().any(|regex| regex.is_match(file_path));
+ let is_match = file_path.contains("node_modules") && self.node_modules_regexes.iter().any(|regex| regex.is_match(file_path));
- let _ = self.watcher.watch(resource.0.path().to_path_buf().as_path(), notify::RecursiveMode::NonRecursive);
+ if let Err(e) = self.watcher.watch(resource.0.path().to_path_buf().as_path(), notify::RecursiveMode::NonRecursive) {
+ eprintln!("Failed to watch path: {:?}", e);
+ }
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.
if !self.node_modules_regexes.is_empty() { | |
let file_path = resource.0.path().to_str().unwrap(); | |
let is_match = file_path.contains("node_modules") | |
&& self | |
.node_modules_regexes | |
.iter() | |
.any(|regex| regex.is_match(file_path)); | |
if is_match { | |
let _ = self.watcher.watch( | |
resource.0.path().to_path_buf().as_path(), | |
notify::RecursiveMode::NonRecursive, | |
); | |
} | |
} | |
if !self.node_modules_regexes.is_empty() { | |
let file_path = resource.0.path().to_str().unwrap(); | |
let is_match = file_path.contains("node_modules") && self.node_modules_regexes.iter().any(|regex| regex.is_match(file_path)); | |
if is_match { | |
if let Err(e) = self.watcher.watch( | |
resource.0.path().to_path_buf().as_path(), | |
notify::RecursiveMode::NonRecursive, | |
) { | |
eprintln!("Failed to watch path: {:?}", e); | |
} | |
} | |
} |
watch module in node_modules directly make too many files os error
add config node_modules_regexes to watch node_modules
Summary by CodeRabbit
Summary by CodeRabbit
node_modules
目录中路径的正则表达式匹配功能,用户可以在配置中指定要忽略的文件和目录,从而提高文件监视的性能和相关性。BuildParams
结构,新增可选属性nodeModulesRegexes
,允许用户定义路径匹配模式以优化构建过程。