fix(ext/node): throw ERR_INVALID_ARG_VALUE when rmdir gets options.recursive

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);

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": {},

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,15 +34,15 @@ 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"));
     using _file2 = Deno.createSync(join(dir, "file2.txt"));
     Deno.mkdirSync(join(dir, "some_dir"));
     using _file = Deno.createSync(join(dir, "some_dir", "file.txt"));
     await new Promise<void>((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);
   },
 });

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<string, number> = {
-  "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,