feat(turbopack): support resolve windows absolute path with project_root #103
Conversation
Summary of ChangesHello @fireairforce, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在增强 Turbopack 在 Windows 环境下的兼容性，通过引入对 Windows 绝对路径的解析能力，解决了开发者在导入模块时可能遇到的路径问题。这一改进使得 Turbopack 能够更智能地处理不同操作系统下的文件引用，同时通过限制解析范围在项目根目录内，确保了安全性和一致性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for resolving absolute Windows paths within the project root in Turbopack. The overall approach is sound and correctly handles the new case while providing a more informative error message for paths outside the project root. However, the implementation introduces a significant amount of code duplication for path resolution. I've suggested a refactoring to use an existing helper function, which will improve code maintainability. Other changes are minor stylistic improvements.
| // Successfully converted - resolve as a raw path | ||
| let mut results = Vec::new(); | ||
| let unix_path = &fs_path.path; | ||
| let pattern = Pattern::Constant(unix_path.clone()); | ||
| let matches = read_matches( | ||
| root_path.clone(), | ||
| rcstr!(""), | ||
| false, | ||
| Pattern::new(pattern).resolve().await?, | ||
| ) | ||
| .await?; | ||
|
|
||
| for m in matches.iter() { | ||
| match m { | ||
| PatternMatch::File(matched_pattern, path) => { | ||
| results.push( | ||
| resolved( | ||
| RequestKey::new(matched_pattern.clone()), | ||
| path.clone(), | ||
| lookup_path.clone(), | ||
| request, | ||
| options_value, | ||
| options, | ||
| query.clone(), | ||
| fragment.clone(), | ||
| ) | ||
| .await?, | ||
| ); | ||
| } | ||
| PatternMatch::Directory(matched_pattern, path) => { | ||
| results.push( | ||
| resolve_into_folder(path.clone(), options) | ||
| .with_request(matched_pattern.clone()), | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return Ok(merge_results(results)); |
There was a problem hiding this comment.
This block of code for resolving the path appears to duplicate logic that is already present in resolve_raw_request_into_results. To improve maintainability and reduce code duplication, consider refactoring this to call resolve_raw_request_into_results instead.
// Successfully converted - resolve as a raw path
let raw_path_vc = Vc::cell(RequestContent::new(Value::new(fs_path.path.into())));
let results = resolve_raw_request_into_results(
lookup_path,
raw_path_vc,
query,
fragment,
options,
options_value,
request,
)
.await?;
return Ok(merge_results(results));
给 turbopack-resolve 支持解析 windows 下的绝对路径:
如果绝对路径里面包含了项目的 project_path 支持解析，如果解析到项目路径外了，这种情况不支持解析。
ref issue: utooland/utoo#2479