Skip to content

Commit

Permalink
fs: runtime deprecate rmdir recursive option
Browse files Browse the repository at this point in the history
  • Loading branch information
aduh95 committed Feb 23, 2021
1 parent aecae19 commit d902f52
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 42 deletions.
14 changes: 10 additions & 4 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -2668,19 +2668,25 @@ The [`crypto.Certificate()` constructor][] is deprecated. Use
### DEP0147: `fs.rmdir(path, { recursive: true })`
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302
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: Runtime

In future versions of Node.js, `fs.rmdir(path, { recursive: true })` will throw
if `path` does not exist or is a file.
Use `fs.rm(path, { recursive: true, force: true })` instead.
In future versions of Node.js, `recursive` option will be ignored for
`fs.rmdir`, `fs.rmdirSync`, and `fs.promises.rmdir`.

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.

### DEP0148: Folder mappings in `"exports"` (trailing `"/"`)
<!-- YAML
Expand Down
33 changes: 21 additions & 12 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,10 @@ Renames `oldPath` to `newPath`.
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302
description: The `recursive` option is deprecated, using it triggers a
deprecation warning.
- version:
- v13.3.0
- v12.16.0
Expand All @@ -1070,7 +1074,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 All @@ -1084,9 +1088,8 @@ error on POSIX.
Setting `recursive` to `true` results in behavior similar to the Unix command
`rm -rf`: an error will not be raised for paths that do not exist, and paths
that represent files will be deleted. The permissive behavior of the
`recursive` option is deprecated, `ENOTDIR` and `ENOENT` will be thrown in
the future.
that represent files will be deleted. The `recursive` option is deprecated,
`ENOTDIR` and `ENOENT` will be thrown in the future.
### `fsPromises.rm(path[, options])`
<!-- YAML
Expand Down Expand Up @@ -3131,6 +3134,10 @@ rename('oldFile.txt', 'newFile.txt', (err) => {
<!-- YAML
added: v0.0.2
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302
description: The `recursive` option is deprecated, using it triggers a
deprecation warning.
- version:
- v13.3.0
- v12.16.0
Expand Down Expand Up @@ -3167,7 +3174,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 All @@ -3182,9 +3189,8 @@ Windows and an `ENOTDIR` error on POSIX.

Setting `recursive` to `true` results in behavior similar to the Unix command
`rm -rf`: an error will not be raised for paths that do not exist, and paths
that represent files will be deleted. The permissive behavior of the
`recursive` option is deprecated, `ENOTDIR` and `ENOENT` will be thrown in
the future.
that represent files will be deleted. The `recursive` option is deprecated,
`ENOTDIR` and `ENOENT` will be thrown in the future.

### `fs.rm(path[, options], callback)`
<!-- YAML
Expand Down Expand Up @@ -4751,6 +4757,10 @@ See the POSIX rename(2) documentation for more details.
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302
description: The `recursive` option is deprecated, using it triggers a
deprecation warning.
- version:
- v13.3.0
- v12.16.0
Expand Down Expand Up @@ -4779,7 +4789,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 All @@ -4791,9 +4801,8 @@ on Windows and an `ENOTDIR` error on POSIX.
Setting `recursive` to `true` results in behavior similar to the Unix command
`rm -rf`: an error will not be raised for paths that do not exist, and paths
that represent files will be deleted. The permissive behavior of the
`recursive` option is deprecated, `ENOTDIR` and `ENOENT` will be thrown in
the future.
that represent files will be deleted. The `recursive` option is deprecated,
`ENOTDIR` and `ENOENT` will be thrown in the future.
### `fs.rmSync(path[, options])`
<!-- YAML
Expand Down
3 changes: 3 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ const internalUtil = require('internal/util');
const {
copyObject,
Dirent,
emitRecursiveRmdirWarning,
getDirents,
getOptions,
getValidatedFd,
Expand Down Expand Up @@ -888,6 +889,7 @@ function rmdir(path, options, callback) {
path = pathModule.toNamespacedPath(getValidatedPath(path));

if (options?.recursive) {
emitRecursiveRmdirWarning();
validateRmOptions(
path,
{ ...options, force: true },
Expand All @@ -912,6 +914,7 @@ function rmdirSync(path, options) {
path = getValidatedPath(path);

if (options?.recursive) {
emitRecursiveRmdirWarning();
options = validateRmOptionsSync(path, { ...options, force: true }, true);
lazyLoadRimraf();
return rimrafSync(pathModule.toNamespacedPath(path), options);
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const { isArrayBufferView } = require('internal/util/types');
const { rimrafPromises } = require('internal/fs/rimraf');
const {
copyObject,
emitRecursiveRmdirWarning,
getDirents,
getOptions,
getStatsFromBinding,
Expand Down Expand Up @@ -489,6 +490,7 @@ async function rmdir(path, options) {
options = validateRmdirOptions(options);

if (options.recursive) {
emitRecursiveRmdirWarning();
return rimrafPromises(path, options);
}

Expand Down
24 changes: 6 additions & 18 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -704,18 +704,11 @@ const validateRmOptions = hideStackFrames((path, options, warn, callback) => {
lazyLoadFs().stat(path, (err, stats) => {
if (err) {
if (options.force && err.code === 'ENOENT') {
if (warn) {
emitPermissiveRmdirWarning();
}
return callback(null, options);
}
return callback(err, options);
}

if (warn && !stats.isDirectory()) {
emitPermissiveRmdirWarning();
}

if (stats.isDirectory() && !options.recursive) {
return callback(new ERR_FS_EISDIR({
code: 'EISDIR',
Expand All @@ -737,10 +730,6 @@ const validateRmOptionsSync = hideStackFrames((path, options, warn) => {
const isDirectory = lazyLoadFs()
.statSync(path, { throwIfNoEntry: !options.force })?.isDirectory();

if (warn && !isDirectory) {
emitPermissiveRmdirWarning();
}

if (isDirectory && !options.recursive) {
throw new ERR_FS_EISDIR({
code: 'EISDIR',
Expand All @@ -755,18 +744,16 @@ const validateRmOptionsSync = hideStackFrames((path, options, warn) => {
return options;
});

let permissiveRmdirWarned = false;

function emitPermissiveRmdirWarning() {
if (!permissiveRmdirWarned) {
let recursiveRmdirWarned = process.noDeprecation;
function emitRecursiveRmdirWarning() {
if (!recursiveRmdirWarned) {
process.emitWarning(
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will throw if path does not exist or is a file. Use fs.rm(path, ' +
'{ recursive: true, force: true }) instead',
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DeprecationWarning',
'DEP0147'
);
permissiveRmdirWarned = true;
recursiveRmdirWarned = true;
}
}

Expand Down Expand Up @@ -851,6 +838,7 @@ module.exports = {
BigIntStats, // for testing
copyObject,
Dirent,
emitRecursiveRmdirWarning,
getDirent,
getDirents,
getOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ tmpdir.refresh();
common.expectWarning(
'DeprecationWarning',
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will throw if path does not exist or is a file. Use fs.rm(path, ' +
'{ recursive: true, force: true }) instead',
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DEP0147'
);
fs.rmdirSync(path.join(tmpdir.path, 'noexist.txt'), { recursive: true });
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-fs-rmdir-recursive-sync-warns-on-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ tmpdir.refresh();
common.expectWarning(
'DeprecationWarning',
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will throw if path does not exist or is a file. Use fs.rm(path, ' +
'{ recursive: true, force: true }) instead',
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DEP0147'
);
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-fs-rmdir-recursive-warns-not-found.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ tmpdir.refresh();
common.expectWarning(
'DeprecationWarning',
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will throw if path does not exist or is a file. Use fs.rm(path, ' +
'{ recursive: true, force: true }) instead',
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DEP0147'
);
fs.rmdir(
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-fs-rmdir-recursive-warns-on-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ tmpdir.refresh();
common.expectWarning(
'DeprecationWarning',
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
'will throw if path does not exist or is a file. Use fs.rm(path, ' +
'{ recursive: true, force: true }) instead',
'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DEP0147'
);
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-fs-rmdir-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ const fs = require('fs');
const path = require('path');
const { validateRmdirOptions } = require('internal/fs/utils');

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

tmpdir.refresh();

let count = 0;
Expand Down

0 comments on commit d902f52

Please sign in to comment.