Fix ProgressData#deletedCount#142
Conversation
|
// @jopemachine |
jopemachine
left a comment
There was a problem hiding this comment.
Thanks for correcting this :)
It was my mistake when occurred in changing the call order of onProgress.
The below function call should be removed since the last element will be called twice by the line.
Lines 99 to 103 in 7fbeca0
There was a problem hiding this comment.
And I think using fileIndex through mapper's argument here was another mistake.
deletedCount should be in ascending order semantically, but because the mapper function is async, it will be called in order mapper's promise resolve.
It means the below test will break.
Line 381 in 7fbeca0
I think probably this should be fixed with like below codes.
let fileIndex = 1;
const mapper = async (file) => {
// ...
onProgress({
totalCount: files.length,
deletedCount: fileIndex,
percent: fileIndex / files.length
});
++fileIndex;
// ...
}
let count = 0
const mapper = async (file, fileIndex) => {
file = path.resolve(cwd, file);
if (!force) {
safeCheck(file, cwd);
}
if (!dryRun) {
await rimrafP(file, rimrafOptions);
}
count += 1
onProgress({
totalCount: files.length,
deletedCount: count,
percent: count / files.length
});
return file;
};Agree your point, but i think we should replace |
I agree the variable needs be renamed, but I think |
|
In the doc, the {
totalFiles: number,
deletedFiles: number,
percent: number
}
|
|
Thanks for correcting. That should be |
|
@sindresorhus |
|
Thanks, everyone :) |
deletedCountshould start from1