Remove mkdirp and rimraf in favor of Node.js builtins #17696
Conversation
gulpfile.mjs
Outdated
| safeSpawnSync("git", ["clone", "--depth", "1", DIST_REPO_URL, DIST_DIR]); | ||
|
|
||
| console.log(); | ||
| console.log("### Overwriting all files"); | ||
| rimraf.sync(path.join(DIST_DIR, "*")); | ||
| fs.rmSync(DIST_DIR, { recursive: true, force: true }); |
There was a problem hiding this comment.
That's the only line I'm not sure of
There was a problem hiding this comment.
If you run gulp dist-pre locally and compare the result with/without this patch you'll see that this change breaks things, since the output is no longer identical.
Hence this patch would reintroduce issue #15975, note in particular #15975 (comment).
There was a problem hiding this comment.
Hmmm, trying something else. I don't see the difference in outputs now, but I need a fresh brain to be sure.
gulpfile.mjs
Outdated
| safeSpawnSync("git", ["clone", "--depth", "1", DIST_REPO_URL, DIST_DIR]); | ||
|
|
||
| console.log(); | ||
| console.log("### Overwriting all files"); | ||
| rimraf.sync(path.join(DIST_DIR, "*")); | ||
| fs.rmSync(DIST_DIR, { recursive: true, force: true }); |
There was a problem hiding this comment.
If you run gulp dist-pre locally and compare the result with/without this patch you'll see that this change breaks things, since the output is no longer identical.
Hence this patch would reintroduce issue #15975, note in particular #15975 (comment).
|
It's probably a good idea to split this into two separate PRs, one for |
|
Would it be possible to have few tests to check that we aren't breaking something again ? |
|
Closing this for now, since there's not been any updates for a couple of weeks and this cannot land in its current state (needs careful testing and no fixup commits for a start). |
In Node.js v10.12.0,
recursiveoption was added tofs.mkdirSync. This option allows you to create a directory and all its parent directories if they do not exist.In Node.js v14.14.0,
fs.rmSyncwas added. It's a built-in module that removes files and directories. Therecursiveoption is used to remove directories and their contents.Since pdf.js repository requires Node.js 18 and up, it's safe to assume
mkdirpandrimrafcan be safely replaced, thus reducing the number of dependencies in this project.