-
Notifications
You must be signed in to change notification settings - Fork 107
[WIP] refactor: use turbo-fs-task to implement output.clean #1935
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
base: next
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR refactors the output directory cleaning functionality to use turbo-fs-task operations instead of direct filesystem operations. The change replaces standard library fs operations with turbo-tasks-fs abstractions for better integration with the async task system.
- Removes direct filesystem operations and replaces them with turbo-tasks-fs operations
- Implements new
delete_fileandclean_directoryfunctions as turbo-tasks operations - Updates the project emission flow to use the new cleaning mechanism
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let dist_root = self.dist_root(); | ||
| tracing::info!("Cleaning dist directory: {}", dist_root.await?.path); | ||
|
|
||
| let _ = clean_directory(dist_root.to_resolved().await?).connect().resolve().await?; |
Copilot
AI
Sep 2, 2025
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.
The result of clean_directory is being discarded with let _, which ignores potential errors. Since this is a cleaning operation that could fail, the error should be handled properly, either by logging a warning or propagating the error.
| let _ = clean_directory(dist_root.to_resolved().await?).connect().resolve().await?; | |
| clean_directory(dist_root.to_resolved().await?).connect().resolve().await?; |
| let _ = delete_file(entry_path.to_resolved().await?).connect().resolve().await?; | ||
| Ok::<(), anyhow::Error>(()) | ||
| } | ||
| turbo_tasks_fs::DirectoryEntry::Directory(_) => { | ||
| let _ = clean_directory(entry_path.to_resolved().await?).connect().resolve().await?; |
Copilot
AI
Sep 2, 2025
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.
The results of delete_file and clean_directory operations are being discarded with let _, which ignores potential errors during the cleaning process. These operations should handle errors properly or at least log failures since silent failures during cleanup could leave the directory in an inconsistent state.
| let _ = delete_file(entry_path.to_resolved().await?).connect().resolve().await?; | ||
| Ok::<(), anyhow::Error>(()) | ||
| } | ||
| turbo_tasks_fs::DirectoryEntry::Directory(_) => { | ||
| let _ = clean_directory(entry_path.to_resolved().await?).connect().resolve().await?; |
Copilot
AI
Sep 2, 2025
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.
The results of delete_file and clean_directory operations are being discarded with let _, which ignores potential errors during the cleaning process. These operations should handle errors properly or at least log failures since silent failures during cleanup could leave the directory in an inconsistent state.
| for task in delete_tasks { | ||
| task.await?; | ||
| } |
Copilot
AI
Sep 2, 2025
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.
The delete operations are being executed sequentially in a for loop, which could be slow for directories with many files. Consider using futures::future::try_join_all(delete_tasks) or similar to execute the delete operations concurrently for better performance.
2ab3074 to
3345bb8
Compare
WIP