[Fix] sync/async: fix preserveSymlinks option

Fixes #177.

Author: ljharb

.gitignore

@@ -1,5 +1,6 @@
 # gitignore
 node_modules
+**/node_modules
 
 # Only apps should have lockfiles
 npm-shrinkwrap.json

.travis.yml

@@ -35,6 +35,8 @@ matrix:
   include:
     - node_js: "lts/*"
       env: PRETEST=true
+    - node_js: "lts/*"
+      env: POSTTEST=true
     - node_js: "11.3"
       env: TEST=true ALLOW_FAILURE=true
     - node_js: "11.2"

lib/async.js

@@ -51,18 +51,34 @@ module.exports = function resolve(x, options, callback) {
 
     opts.paths = opts.paths || [];
 
-    if (opts.basedir) {
-        var basedirError = new TypeError('Provided basedir "' + opts.basedir + '" is not a directory');
-        isDirectory(opts.basedir, function (err, result) {
-            if (err) return cb(err);
-            if (!result) { return cb(basedirError); }
-            validBasedir();
+    // ensure that `basedir` is an absolute path at this point, resolving against the process' current working directory
+    var absoluteStart = path.resolve(basedir);
+
+    if (!opts || !opts.preserveSymlinks) {
+        fs.realpath(absoluteStart, function (realPathErr, realStart) {
+            if (realPathErr && realPathErr.code !== 'ENOENT') cb(err);
+            else validateBasedir(realPathErr ? absoluteStart : realStart);
         });
     } else {
-        validBasedir();
+        validateBasedir(absoluteStart);
+    }
+
+    function validateBasedir(basedir) {
+        if (opts.basedir) {
+            var dirError = new TypeError('Provided basedir "' + basedir + '" is not a directory' + (opts.preserveSymlinks ? '' : ', or a symlink to a directory'));
+            dirError.code = 'INVALID_BASEDIR';
+            isDirectory(basedir, function (err, result) {
+                if (err) return cb(err);
+                if (!result) { return cb(dirError); }
+                validBasedir(basedir);
+            });
+        } else {
+            validBasedir(basedir);
+        }
     }
+
     var res;
-    function validBasedir() {
+    function validBasedir(basedir) {
         if ((/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/).test(x)) {
             res = path.resolve(basedir, x);
             if (x === '..' || x.slice(-1) === '/') res += '/';

lib/node-modules-paths.js

@@ -1,5 +1,4 @@
 var path = require('path');
-var fs = require('fs');
 var parse = path.parse || require('path-parse');
 
 var getNodeModulesDirs = function getNodeModulesDirs(absoluteStart, modules) {
@@ -29,28 +28,15 @@ module.exports = function nodeModulesPaths(start, opts, request) {
         ? [].concat(opts.moduleDirectory)
         : ['node_modules'];
 
-    // ensure that `start` is an absolute path at this point, resolving against the process' current working directory
-    var absoluteStart = path.resolve(start);
-
-    if (!opts || !opts.preserveSymlinks) {
-        try {
-            absoluteStart = fs.realpathSync(absoluteStart);
-        } catch (err) {
-            if (err.code !== 'ENOENT') {
-                throw err;
-            }
-        }
-    }
-
     if (opts && typeof opts.paths === 'function') {
         return opts.paths(
             request,
-            absoluteStart,
-            function () { return getNodeModulesDirs(absoluteStart, modules); },
+            start,
+            function () { return getNodeModulesDirs(start, modules); },
             opts
         );
     }
 
-    var dirs = getNodeModulesDirs(absoluteStart, modules);
+    var dirs = getNodeModulesDirs(start, modules);
     return opts && opts.paths ? dirs.concat(opts.paths) : dirs;
 };

lib/sync.js

@@ -41,19 +41,34 @@ module.exports = function (x, options) {
 
     opts.paths = opts.paths || [];
 
-    if (opts.basedir && !isDirectory(opts.basedir)) {
-        throw new TypeError('Provided basedir "' + opts.basedir + '" is not a directory');
+    // ensure that `basedir` is an absolute path at this point, resolving against the process' current working directory
+    var absoluteStart = path.resolve(basedir);
+
+    if (!opts.preserveSymlinks) {
+        try {
+            absoluteStart = fs.realpathSync(absoluteStart);
+        } catch (realPathErr) {
+            if (realPathErr.code !== 'ENOENT') {
+                throw realPathErr;
+            }
+        }
+    }
+
+    if (opts.basedir && !isDirectory(absoluteStart)) {
+        var dirError = new TypeError('Provided basedir "' + opts.basedir + '" is not a directory' + (opts.preserveSymlinks ? '' : ', or a symlink to a directory'));
+        dirError.code = 'INVALID_BASEDIR';
+        throw dirError;
     }
 
     if ((/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/).test(x)) {
-        var res = path.resolve(basedir, x);
+        var res = path.resolve(absoluteStart, x);
         if (x === '..' || x.slice(-1) === '/') res += '/';
         var m = loadAsFileSync(res) || loadAsDirectorySync(res);
         if (m) return m;
     } else if (core[x]) {
         return x;
     } else {
-        var n = loadNodeModulesSync(x, basedir);
+        var n = loadNodeModulesSync(x, absoluteStart);
         if (n) return n;
     }
 

package.json

@@ -18,7 +18,9 @@
     "lint": "eslint .",
     "tests-only": "tape test/*.js",
     "pretest": "npm run lint",
-    "test": "npm run --silent tests-only"
+    "test": "npm run --silent tests-only",
+    "posttest": "npm run test:multirepo",
+    "test:multirepo": "cd ./test/resolver/multirepo && npm install && npm test"
   },
   "devDependencies": {
     "@ljharb/eslint-config": "^13.0.0",

test/node_path.js

@@ -60,7 +60,7 @@ test('$NODE_PATH', function (t) {
         paths: [
             'node_path'
         ],
-        basedir: 'node_path/x',
+        basedir: path.join(__dirname, 'node_path/x'),
         isDirectory: isDir
     }, function (err, res) {
         var root = require('tap/package.json').main;

test/resolver.js

@@ -353,15 +353,16 @@ test('async dot slash main', function (t) {
 });
 
 test('not a directory', function (t) {
-    t.plan(5);
+    t.plan(6);
     var path = './foo';
     resolve(path, { basedir: __filename }, function (err, res, pkg) {
         t.ok(err, 'a non-directory errors');
         t.equal(arguments.length, 1);
         t.equal(res, undefined);
         t.equal(pkg, undefined);
 
-        t.equal(err && err.message, 'Provided basedir "' + __filename + '" is not a directory');
+        t.equal(err && err.message, 'Provided basedir "' + __filename + '" is not a directory, or a symlink to a directory');
+        t.equal(err && err.code, 'INVALID_BASEDIR');
     });
 });
 

test/resolver/multirepo/.npmrc

@@ -0,0 +1 @@
+package-lock=false

test/resolver/multirepo/lerna.json

@@ -0,0 +1,6 @@
+{
+  "packages": [
+    "packages/*"
+  ],
+  "version": "0.0.0"
+}