fs: add directory autodetection to fsPromises.symlink()

PR-URL: #42894 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Oct 3, 2022 · a4fa526 · a4fa526
1 parent 50aac70
commit a4fa526
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 4 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -1463,18 +1463,27 @@ changes:
 
 <!-- YAML
 added: v10.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42894
+    description: If the `type` argument is `null` or omitted, Node.js will
+                 autodetect `target` type and automatically
+                 select `dir` or `file`.
+
 -->
 
 * `target` {string|Buffer|URL}
 * `path` {string|Buffer|URL}
-* `type` {string} **Default:** `'file'`
+* `type` {string|null} **Default:** `null`
 * Returns: {Promise} Fulfills with `undefined` upon success.
 
 Creates a symbolic link.
 
 The `type` argument is only used on Windows platforms and can be one of `'dir'`,
-`'file'`, or `'junction'`. Windows junction points require the destination path
-to be absolute. When using `'junction'`, the `target` argument will
+`'file'`, or `'junction'`. If the `type` argument is not a string, Node.js will
+autodetect `target` type and use `'file'` or `'dir'`. If the `target` does not
+exist, `'file'` will be used. Windows junction points require the destination
+path to be absolute. When using `'junction'`, the `target` argument will
 automatically be normalized to absolute path.
 
 ### `fsPromises.truncate(path[, len])`

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -117,6 +117,8 @@ const {
 const getDirectoryEntriesPromise = promisify(getDirents);
 const validateRmOptionsPromise = promisify(validateRmOptions);
 
+const isWindows = process.platform === 'win32';
+
 let cpPromises;
 function lazyLoadCpPromises() {
   return cpPromises ??= require('internal/fs/cp/cp').cpFn;
@@ -714,7 +716,16 @@ async function readlink(path, options) {
 }
 
 async function symlink(target, path, type_) {
-  const type = (typeof type_ === 'string' ? type_ : null);
+  let type = (typeof type_ === 'string' ? type_ : null);
+  if (isWindows && type === null) {
+    try {
+      const absoluteTarget = pathModule.resolve(`${path}`, '..', `${target}`);
+      type = (await stat(absoluteTarget)).isDirectory() ? 'dir' : 'file';
+    } catch {
+      // Default to 'file' if path is invalid or file does not exist
+      type = 'file';
+    }
+  }
   target = getValidatedPath(target, 'target');
   path = getValidatedPath(path);
   return binding.symlink(preprocessSymlinkDestination(target, type, path),

diff --git a/test/parallel/test-fs-symlink-dir.js b/test/parallel/test-fs-symlink-dir.js
@@ -12,6 +12,7 @@ if (!common.canCreateSymLink())
 const assert = require('assert');
 const path = require('path');
 const fs = require('fs');
+const fsPromises = fs.promises;
 
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
@@ -36,11 +37,18 @@ function testAsync(target, path) {
   }));
 }
 
+async function testPromises(target, path) {
+  await fsPromises.symlink(target, path);
+  fs.readdirSync(path);
+}
+
 for (const linkTarget of linkTargets) {
   fs.mkdirSync(path.resolve(tmpdir.path, linkTarget));
   for (const linkPath of linkPaths) {
     testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
     testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
+    testPromises(linkTarget, `${linkPath}-${path.basename(linkTarget)}-promises`)
+      .then(common.mustCall());
   }
 }
 
@@ -57,10 +65,17 @@ for (const linkTarget of linkTargets) {
     }));
   }
 
+  async function testPromises(target, path) {
+    await fsPromises.symlink(target, path);
+    assert(!fs.existsSync(path));
+  }
+
   for (const linkTarget of linkTargets.map((p) => p + '-broken')) {
     for (const linkPath of linkPaths) {
       testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
       testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
+      testPromises(linkTarget, `${linkPath}-${path.basename(linkTarget)}-promises`)
+        .then(common.mustCall());
     }
   }
 }