Skip to content

Commit

Permalink
Use fs.rm() instead of rimraf
Browse files Browse the repository at this point in the history
A regression occurred in 0.2.2 which can be reproduced using:

```js
const assert = require('assert');
const tmp = require('tmp');
tmp.dir({ unsafeCleanup: true }, (err, path, cleanup) => {
  assert.ifError(err);
  cleanup(assert.ifError);
});
```
This works with 0.2.1.  With 0.2.2 it fails with:

    /path/to/tmp/node-tmp/lib/tmp.js:358
            return removeFunction(fileOrDirName, next || function() {});
                   ^

    TypeError: removeFunction is not a function
        at _cleanupCallback (/path/to/tmp/node-tmp/lib/tmp.js:358:16)
        at /path/to/tmp/node-tmp/repro.js:5:3
        at _dirCreated (/path/to/tmp/node-tmp/lib/tmp.js:207:7)
        at FSReqCallback.oncomplete (node:fs:192:23)

This occurs because 00bb5b2 upgraded
the rimraf dependency from ^3.0.0 to ^5.0.5 without handling the change
to a `Promise`-based API in 4.0.0
(isaacs/rimraf@a71e7f9)
or the removal of the default export in 5.0.0
(isaacs/rimraf@c7a3fd4).

This commit fixes the issue by dropping the `rimraf` dependency in favor
of `fs.rm({recursive: true})`.

Fixes: 00bb5b2 ("Update rimraf and drop old Node compatibility")
Fixes: #295
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
  • Loading branch information
kevinoid committed Feb 28, 2024
1 parent 9fcf3ce commit 4e89567
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
35 changes: 32 additions & 3 deletions lib/tmp.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const os = require('os');
const path = require('path');
const crypto = require('crypto');
const _c = { fs: fs.constants, os: os.constants };
const rimraf = require('rimraf');

/*
* The working inner variables.
Expand Down Expand Up @@ -43,12 +42,42 @@ const
_removeObjects = [],

// API change in fs.rmdirSync leads to error when passing in a second parameter, e.g. the callback
FN_RMDIR_SYNC = fs.rmdirSync.bind(fs),
FN_RIMRAF_SYNC = rimraf.sync;
FN_RMDIR_SYNC = fs.rmdirSync.bind(fs);

let
_gracefulCleanup = false;

/**
* Recursively remove a directory and its contents.
*
* @param {string} dirPath path of directory to remove
* @param {Function} callback
*/
function rimraf(dirPath, callback) {
// fs.rm() was added in Node.js 14.14.
// Use deprecated rmdir({recursive: true}) on older versions.
if (!fs.rm) {
return fs.rmdir(dirPath, { recursive: true }, callback);
}

return fs.rm(dirPath, { recursive: true }, callback);
}

/**
* Recursively remove a directory and its contents, synchronously.
*
* @param {string} dirPath path of directory to remove
*/
function FN_RIMRAF_SYNC(dirPath) {
// fs.rm() was added in Node.js 14.14.
// Use deprecated rmdir({recursive: true}) on older versions.
if (!fs.rm) {
return fs.rmdirSync(dirPath, { recursive: true });
}

return fs.rmSync(dirPath, { recursive: true });
}

/**
* Gets a temporary file name.
*
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
"node": ">=14"
},
"dependencies": {
"rimraf": "^5.0.5"
},
"devDependencies": {
"eslint": "^6.3.0",
Expand Down

0 comments on commit 4e89567

Please sign in to comment.