[WIP] refactor: use turbo-fs-task to implement output.clean#1935
[WIP] refactor: use turbo-fs-task to implement output.clean#1935fireairforce wants to merge 1 commit intonextfrom
Conversation
xusd320
changed the title
There was a problem hiding this comment.
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?; |
There was a problem hiding this comment.
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?; |
There was a problem hiding this comment.
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?; |
There was a problem hiding this comment.
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?; | ||
| } |
There was a problem hiding this comment.
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.
xusd320
force-pushed
the
next
branch
5 times, most recently
from
2ab3074 to
3345bb8
Compare
xusd320
force-pushed
the
next
branch
2 times, most recently
from
f77808c to
567e5ae
Compare
WIP