From 6b821a97c06061781a6e656798ef3ea55b3f8132 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 8 Feb 2021 12:24:06 +0100 Subject: [PATCH] fixup! fs: remove permissive rmdir recursive --- doc/api/deprecations.md | 12 +++++++----- doc/api/fs.md | 4 +++- lib/internal/fs/promises.js | 6 +++++- lib/internal/fs/utils.js | 18 ++++++++++++++++++ test/parallel/test-fs-rmdir-recursive.js | 7 +++++++ 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 8bcd3200ca0c7e..ca3437a868b148 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -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. diff --git a/doc/api/fs.md b/doc/api/fs.md index e42bb5f5610c3d..96a287bc11df1a 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -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`. @@ -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`. @@ -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`. diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 435ebdc1c11403..30d45ce6ec1193 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -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, @@ -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); diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 7aa174d3d3f026..660c1bb80ba69e 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -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) @@ -753,6 +766,10 @@ const validateRmdirOptions = hideStackFrames( validateInt32(options.retryDelay, 'options.retryDelay', 0); validateUint32(options.maxRetries, 'options.maxRetries'); + if (options.recursive) { + emitRecursiveRmdirWarning(); + } + return options; }); @@ -822,6 +839,7 @@ module.exports = { BigIntStats, // for testing copyObject, Dirent, + emitRecursiveRmdirWarning, getDirent, getDirents, getOptions, diff --git a/test/parallel/test-fs-rmdir-recursive.js b/test/parallel/test-fs-rmdir-recursive.js index 2077c5dcce3dfc..f31248ed18e085 100644 --- a/test/parallel/test-fs-rmdir-recursive.js +++ b/test/parallel/test-fs-rmdir-recursive.js @@ -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;