Skip to content
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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 25, 2024
Merged

feat: add regexes from node_modules watch #1439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85d12f5
feat: add regexes from node_modules watch
Jinbao1001 Jul 23, 2024
4e5eff8
fix: self add node_modules_regexes
Jinbao1001 Jul 23, 2024
346013c
Merge remote-tracking branch 'origin/master' into feat/regex-node_mod…
Jinbao1001 Jul 23, 2024
b8abf47
chore: ignorePath to Options
Jinbao1001 Jul 24, 2024
34538fb
chore: judge contains node_modules first
Jinbao1001 Jul 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/binding/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub struct BuildParams {
};
watch?: {
ignoredPaths?: string[];
nodeModulesRegexes?: string[];
};
}"#)]
pub config: serde_json::Value,
Expand Down
5 changes: 3 additions & 2 deletions crates/mako/src/config/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ pub struct ExperimentalConfig {
#[derive(Deserialize, Serialize, Debug)]
#[serde(rename_all = "camelCase")]
pub struct WatchConfig {
pub ignore_paths: Vec<String>,
pub ignore_paths: Option<Vec<String>>,
pub node_modules_regexes: Option<Vec<String>>,
}

#[derive(Deserialize, Serialize, Debug)]
Expand Down Expand Up @@ -719,7 +720,7 @@ const DEFAULT_CONFIG: &str = r#"
},
"useDefineForClassFields": true,
"emitDecoratorMetadata": false,
"watch": { "ignorePaths": [] },
"watch": { "ignorePaths": [], "nodeModulesRegexes": [] },
"devServer": { "host": "127.0.0.1", "port": 3000 }
}
"#;
Expand Down
28 changes: 28 additions & 0 deletions crates/mako/src/dev/watch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use anyhow::{self, Ok};
use colored::Colorize;
use notify::{self, EventKind, Watcher as NotifyWatcher};
use notify_debouncer_full::DebouncedEvent;
use regex::Regex;
use tracing::debug;

use crate::compiler::Compiler;
Expand All @@ -18,6 +19,7 @@ pub struct Watcher<'a> {
pub compiler: &'a Compiler,
pub watched_files: HashSet<PathBuf>,
pub watched_dirs: HashSet<PathBuf>,
node_modules_regexes: Vec<Regex>,
Copy link
Contributor

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.

Suggested change
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<_>, _>>()?;

}

impl<'a> Watcher<'a> {
Expand All @@ -32,6 +34,16 @@ impl<'a> Watcher<'a> {
compiler,
watched_dirs: HashSet::new(),
watched_files: HashSet::new(),
node_modules_regexes: compiler
.context
.config
.watch
.node_modules_regexes
.clone()
.unwrap_or_default()
.iter()
.map(|s| Regex::new(s).unwrap())
.collect::<Vec<Regex>>(),
Comment on lines +37 to +46
Copy link
Contributor

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.

Suggested change
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<_>, _>>()?,

}
}

Expand All @@ -57,6 +69,20 @@ impl<'a> Watcher<'a> {
dirs.insert(dir);
}
}
if !self.node_modules_regexes.is_empty() {
Jinbao1001 marked this conversation as resolved.
Show resolved Hide resolved
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,
);
}
Comment on lines +79 to +84
Copy link
Contributor

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.

Suggested change
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);
}
}

}
Comment on lines +72 to +85
Copy link
Contributor

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.

Suggested change
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);
}
}
}

}
});
dirs.iter().try_for_each(|dir| {
Expand Down Expand Up @@ -103,6 +129,8 @@ impl<'a> Watcher<'a> {
.config
.watch
.ignore_paths
.as_deref()
.unwrap_or(&[])
.iter()
.map(|p| p.as_str()),
);
Expand Down
1 change: 1 addition & 0 deletions packages/mako/binding.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export interface BuildParams {
};
watch?: {
ignoredPaths?: string[];
nodeModulesRegexes?: string[];
};
};
plugins: Array<JsHooks>;
Expand Down
Loading