diff --git a/ext/node/polyfills/fs.ts b/ext/node/polyfills/fs.ts index 100359344dc6a0..7d7c490126d04b 100644 --- a/ext/node/polyfills/fs.ts +++ b/ext/node/polyfills/fs.ts @@ -59,14 +59,12 @@ import { constants as fsUtilConstants, copyObject, Dirent, - emitRecursiveRmdirWarning, getOptions, getValidatedFd, getValidatedPath, getValidatedPathToString, getValidMode, kMaxUserId, - type RmOptions, Stats, stringToFlags, toUnixTimestamp as _toUnixTimestamp, @@ -139,8 +137,8 @@ import { op_node_statfs_sync, } from "ext:core/ops"; import { - ERR_FS_RMDIR_ENOTDIR, ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, uvException, } from "ext:deno_node/internal/errors.ts"; import { toUnixTimestamp } from "ext:deno_node/internal/fs/utils.mjs"; @@ -1737,26 +1735,6 @@ type rmdirOptions = { type rmdirCallback = (err?: Error) => void; -const rmdirRecursive = - (path: string, callback: rmdirCallback) => - (err: Error | false | null, options?: RmOptions) => { - if (err === false) { - return callback(new ERR_FS_RMDIR_ENOTDIR(path)); - } - if (err) { - return callback(err); - } - - PromisePrototypeThen( - Deno.remove(path, { recursive: options?.recursive }), - (_) => callback(), - (err: Error) => - callback( - denoErrorToNodeError(err, { syscall: "rmdir", path }), - ), - ); - }; - function rmdir( path: string | Buffer | URL, callback: rmdirCallback, @@ -1775,44 +1753,40 @@ function rmdir( callback = options; options = undefined; } - validateFunction(callback, "cb"); - path = getValidatedPathToString(path); - if (options?.recursive) { - emitRecursiveRmdirWarning(); - validateRmOptions( - path, - { ...options, force: false }, - true, - rmdirRecursive(path, callback), - ); - } else { - validateRmdirOptions(options); - PromisePrototypeThen( - op_node_rmdir(path), - (_) => callback(), - (err: Error) => - callback( - denoErrorToNodeError(err, { syscall: "rmdir", path }), - ), + if (options?.recursive !== undefined) { + // The `recursive` option was deprecated and removed in Node. Throw with a + // clear message rather than silently doing the wrong thing. + throw new ERR_INVALID_ARG_VALUE( + "options.recursive", + options.recursive, + "is no longer supported", ); } + + validateFunction(callback, "cb"); + path = getValidatedPathToString(path); + + validateRmdirOptions(options); + PromisePrototypeThen( + op_node_rmdir(path), + (_) => callback(), + (err: Error) => + callback( + denoErrorToNodeError(err, { syscall: "rmdir", path }), + ), + ); } function rmdirSync(path: string | Buffer | URL, options?: rmdirOptions) { path = getValidatedPathToString(path); - if (options?.recursive) { - emitRecursiveRmdirWarning(); - const optionsOrFalse = validateRmOptionsSync(path, { - ...options, - force: false, - }, true); - if (optionsOrFalse === false) { - throw new ERR_FS_RMDIR_ENOTDIR(path); - } - return Deno.removeSync(path, { - recursive: true, - }); + + if (options?.recursive !== undefined) { + throw new ERR_INVALID_ARG_VALUE( + "options.recursive", + options.recursive, + "is no longer supported", + ); } validateRmdirOptions(options); diff --git a/tests/node_compat/config.jsonc b/tests/node_compat/config.jsonc index 0f854f5b2df8d1..49b620ab53e073 100644 --- a/tests/node_compat/config.jsonc +++ b/tests/node_compat/config.jsonc @@ -1143,6 +1143,7 @@ "windows": false }, "parallel/test-fs-rmSync-special-char.js": {}, + "parallel/test-fs-rmdir-recursive-error.js": {}, "parallel/test-fs-rmdir-throws-not-found.js": {}, "parallel/test-fs-rmdir-type-check.js": {}, "parallel/test-fs-sir-writes-alot.js": {}, diff --git a/tests/unit_node/_fs/_fs_rmdir_test.ts b/tests/unit_node/_fs/_fs_rmdir_test.ts index 347a63bec450b8..b8cb331b4a8373 100644 --- a/tests/unit_node/_fs/_fs_rmdir_test.ts +++ b/tests/unit_node/_fs/_fs_rmdir_test.ts @@ -1,6 +1,6 @@ // Copyright 2018-2026 the Deno authors. MIT license. import { assertEquals, fail } from "@std/assert"; -import { rmdir, rmdirSync } from "node:fs"; +import { rm, rmdir, rmdirSync, rmSync } from "node:fs"; import { rmdir as rmdirPromise } from "node:fs/promises"; import { existsSync } from "node:fs"; import { join } from "@std/path"; @@ -34,7 +34,7 @@ Deno.test({ }); Deno.test({ - name: "ASYNC: removing non-empty folder", + name: "ASYNC: removing non-empty folder via rm({ recursive })", async fn() { const dir = Deno.makeTempDirSync(); using _file1 = Deno.createSync(join(dir, "file1.txt")); @@ -42,7 +42,7 @@ Deno.test({ Deno.mkdirSync(join(dir, "some_dir")); using _file = Deno.createSync(join(dir, "some_dir", "file.txt")); await new Promise((resolve, reject) => { - rmdir(dir, { recursive: true }, (err) => { + rm(dir, { recursive: true }, (err) => { if (err) reject(err); resolve(); }); @@ -55,14 +55,14 @@ Deno.test({ }); Deno.test({ - name: "SYNC: removing non-empty folder", + name: "SYNC: removing non-empty folder via rmSync({ recursive })", fn() { const dir = Deno.makeTempDirSync(); using _file1 = Deno.createSync(join(dir, "file1.txt")); using _file2 = Deno.createSync(join(dir, "file2.txt")); Deno.mkdirSync(join(dir, "some_dir")); using _file = Deno.createSync(join(dir, "some_dir", "file.txt")); - rmdirSync(dir, { recursive: true }); + rmSync(dir, { recursive: true }); assertEquals(existsSync(dir), false); }, }); diff --git a/tools/lint_plugins/no_deno_api_in_polyfills.ts b/tools/lint_plugins/no_deno_api_in_polyfills.ts index 08e6affab206a9..5c77b9ecfbdb55 100644 --- a/tools/lint_plugins/no_deno_api_in_polyfills.ts +++ b/tools/lint_plugins/no_deno_api_in_polyfills.ts @@ -13,7 +13,7 @@ // must only go down. Update this object when migrating Deno.* APIs away. // Paths are relative to the repo root. export const EXPECTED_VIOLATIONS: Record = { - "ext/node/polyfills/fs.ts": 53, + "ext/node/polyfills/fs.ts": 51, "ext/node/polyfills/process.ts": 31, "ext/node/polyfills/os.ts": 22, "ext/node/polyfills/internal/child_process.ts": 20,