Skip to content

Commit

Permalink
fixup! fs: remove permissive rmdir recursive
Browse files Browse the repository at this point in the history
  • Loading branch information
aduh95 committed Feb 8, 2021
1 parent a896cbe commit 6b821a9
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 7 deletions.
12 changes: 7 additions & 5 deletions doc/api/deprecations.md
Expand Up @@ -2665,20 +2665,22 @@ The [`crypto.Certificate()` constructor][] is deprecated. Use
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: End-of-Life.
description: Runtime deprecation.
- version: v15.0.0
pr-url: https://github.com/nodejs/node/pull/35562
description: Runtime deprecation.
description: Runtime deprecation for permissive behavior.
- version: v14.14.0
pr-url: https://github.com/nodejs/node/pull/35579
description: Documentation-only deprecation.
-->

Type: End-of-Life
Type: Runtime

`fs.rmdir(path, { recursive: true })`, `fs.rmdirSync(path, { recursive:true })`
and `fs.promises.rmdir(path, { recursive:true })` will throw
if `path` does not exist or is a file.
and `fs.promises.rmdir(path, { recursive:true })` throws if `path` does not
exist or is a file, and emits a warning if path is a directory. In future
versions of Node.js, `recursive` option will be ignored.

Use `fs.rm(path, { recursive: true, force: true })`,
`fs.rmSync(path, { recursive: true, force: true })` or
`fs.promises.rm(path, { recursive: true, force: true })` instead.
Expand Down
4 changes: 3 additions & 1 deletion doc/api/fs.md
Expand Up @@ -3629,6 +3629,7 @@ changes:
option is not `true`. **Default:** `0`.
* `recursive` {boolean} If `true`, perform a recursive directory removal. In
recursive mode, operations are retried on failure. **Default:** `false`.
**Deprecated**.
* `retryDelay` {integer} The amount of time in milliseconds to wait between
retries. This option is ignored if the `recursive` option is not `true`.
**Default:** `100`.
Expand Down Expand Up @@ -3686,7 +3687,7 @@ changes:
option is not `true`. **Default:** `0`.
* `recursive` {boolean} If `true`, perform a recursive directory removal. In
recursive mode, errors are not reported if `path` does not exist, and
operations are retried on failure. **Default:** `false`.
operations are retried on failure. **Default:** `false`. **Deprecated**.
* `retryDelay` {integer} The amount of time in milliseconds to wait between
retries. This option is ignored if the `recursive` option is not `true`.
**Default:** `100`.
Expand Down Expand Up @@ -5711,6 +5712,7 @@ changes:
option is not `true`. **Default:** `0`.
* `recursive` {boolean} If `true`, perform a recursive directory removal. In
recursive mode, operations are retried on failure. **Default:** `false`.
**Deprecated**.
* `retryDelay` {integer} The amount of time in milliseconds to wait between
retries. This option is ignored if the `recursive` option is not `true`.
**Default:** `100`.
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/fs/promises.js
Expand Up @@ -41,7 +41,10 @@ const {
ERR_INVALID_ARG_VALUE,
ERR_METHOD_NOT_IMPLEMENTED,
} = codes;
const { isArrayBufferView } = require('internal/util/types');
const {
isArrayBufferView,
emitRecursiveRmdirWarning,
} = require('internal/util/types');
const { rimrafPromises } = require('internal/fs/rimraf');
const {
copyObject,
Expand Down Expand Up @@ -488,6 +491,7 @@ async function rmdir(path, options) {
options = validateRmdirOptions(options);

if (options.recursive) {
emitRecursiveRmdirWarning();
const stats = await stat(path);
if (stats.isDirectory()) {
return rimrafPromises(path, options);
Expand Down
18 changes: 18 additions & 0 deletions lib/internal/fs/utils.js
Expand Up @@ -741,6 +741,19 @@ const validateRmOptionsSync = hideStackFrames((path, options, expectDir) => {
return options;
});

let recursiveRmdirWarned = process.noDeprecation;
function emitRecursiveRmdirWarning() {
if (!recursiveRmdirWarned) {
process.emitWarning(
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DeprecationWarning',
'DEP0147'
);
recursiveRmdirWarned = true;
}
}

const validateRmdirOptions = hideStackFrames(
(options, defaults = defaultRmdirOptions) => {
if (options === undefined)
Expand All @@ -753,6 +766,10 @@ const validateRmdirOptions = hideStackFrames(
validateInt32(options.retryDelay, 'options.retryDelay', 0);
validateUint32(options.maxRetries, 'options.maxRetries');

if (options.recursive) {
emitRecursiveRmdirWarning();
}

return options;
});

Expand Down Expand Up @@ -822,6 +839,7 @@ module.exports = {
BigIntStats, // for testing
copyObject,
Dirent,
emitRecursiveRmdirWarning,
getDirent,
getDirents,
getOptions,
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-fs-rmdir-recursive.js
Expand Up @@ -7,6 +7,13 @@ const fs = require('fs');
const path = require('path');
const { validateRmdirOptions } = require('internal/fs/utils');

common.expectWarning(
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DeprecationWarning',
'DEP0147'
);

tmpdir.refresh();

let count = 0;
Expand Down

0 comments on commit 6b821a9

Please sign in to comment.