From 6526ffeb3dede2537e2184424554b5b4a20d572a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <miroslav@strongloop.com>
Date: Tue, 7 Apr 2015 10:42:49 +0200
Subject: [PATCH] compiler: code cleanup

Refactor and simplify the internal method tryResolveAppPath.

Fix a bug in the resolver of middleware paths which was discovered
by the refactoring.
---
 lib/compiler.js | 92 +++++++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/lib/compiler.js b/lib/compiler.js
index 5e16c9b..30ad2d0 100644
--- a/lib/compiler.js
+++ b/lib/compiler.js
@@ -322,52 +322,61 @@ function tryResolveAppPath(rootDir, relativePath, resolveOptions) {
    * does not enforce any such restriction when resolving the path */
   resolveOptions = resolveOptions || { strict: false };
 
+  var isModuleRelative = false;
   if (relativePath[0] === '/') {
     fullPath = relativePath;
-  } else if (start === './' || start === '..' || !resolveOptions.strict) {
+  } else if (start === './' || start === '..') {
+    fullPath = path.resolve(rootDir, relativePath);
+  } else if (!resolveOptions.strict) {
+    isModuleRelative = true;
     fullPath = path.resolve(rootDir, relativePath);
   }
 
-  if (fullPath && fs.existsSync(fullPath))
-    return fullPath;
-
-  if (start !== './' && start !== '..') {
-    // Handle module-relative path, e.g. `loopback/common/models`
-
-    // Module.globalPaths is a list of globally configured paths like
-    //   [ env.NODE_PATH values, $HOME/.node_modules, etc. ]
-    // Module._nodeModulePaths(rootDir) returns a list of paths like
-    //   [ rootDir/node_modules, rootDir/../node_modules, etc. ]
-    var modulePaths = Module.globalPaths
-      .concat(Module._nodeModulePaths(rootDir));
-
-    fullPath = modulePaths
-      .map(function(candidateDir) {
-        try {
-          var filePath = path.join(candidateDir, relativePath);
-          filePath = require.resolve(filePath);
-          return filePath;
-        } catch (err) {
-          return filePath;
-        }
-      })
-      .filter(function(candidate) {
-        return fs.existsSync(candidate);
-      })
-      [0];
-
-    if (fullPath)
+  if (fullPath) {
+    // This check is needed to support paths pointing to a directory
+    if (fs.existsSync(fullPath)) {
       return fullPath;
-  } else {
-    // Handle relative path, e.g. `./common/models`
+    }
+
     try {
       fullPath = require.resolve(fullPath);
       return fullPath;
     } catch (err) {
-      debug ('Skipping %s - %s', fullPath, err);
+      if (!isModuleRelative) {
+        debug ('Skipping %s - %s', fullPath, err);
+        return undefined;
+      }
     }
   }
 
+  // Handle module-relative path, e.g. `loopback/common/models`
+
+  // Module.globalPaths is a list of globally configured paths like
+  //   [ env.NODE_PATH values, $HOME/.node_modules, etc. ]
+  // Module._nodeModulePaths(rootDir) returns a list of paths like
+  //   [ rootDir/node_modules, rootDir/../node_modules, etc. ]
+  var modulePaths = Module.globalPaths
+    .concat(Module._nodeModulePaths(rootDir));
+
+  fullPath = modulePaths
+    .map(function(candidateDir) {
+      try {
+        var filePath = path.join(candidateDir, relativePath);
+        filePath = require.resolve(filePath);
+        return filePath;
+      } catch (err) {
+        return filePath;
+      }
+    })
+    .filter(function(candidate) {
+      return fs.existsSync(candidate);
+    })
+    [0];
+
+  if (fullPath)
+    return fullPath;
+
+  debug ('Skipping %s - module not found', fullPath);
   return undefined;
 }
 
@@ -447,6 +456,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
   var pathName = segments[0];
   var fragment = segments[1];
   var middlewarePath = pathName;
+  var opts = { strict: true };
 
   if (fragment) {
     resolved.fragment = fragment;
@@ -458,7 +468,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
   }
 
   if (!fragment) {
-    resolved.sourceFile = resolveAppScriptPath(rootDir, middlewarePath);
+    resolved.sourceFile = resolveAppScriptPath(rootDir, middlewarePath, opts);
     return resolved;
   }
 
@@ -468,7 +478,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
   // function
   var m = require(pathName);
   if (typeof m[fragment] === 'function') {
-    resolved.sourceFile = resolveAppScriptPath(rootDir, middlewarePath);
+    resolved.sourceFile = resolveAppScriptPath(rootDir, middlewarePath, opts);
     return resolved;
   }
 
@@ -486,7 +496,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
 
   for (var ix in candidates) {
     try {
-      resolved.sourceFile = resolveAppScriptPath(rootDir, candidates[ix]);
+      resolved.sourceFile = resolveAppScriptPath(rootDir, candidates[ix], opts);
       delete resolved.fragment;
       return resolved;
     }
@@ -516,7 +526,7 @@ function buildComponentInstructions(rootDir, componentConfig) {
     .filter(function(name) { return !!componentConfig[name]; })
     .map(function(name) {
       return {
-        sourceFile: resolveAppScriptPath(rootDir, name, {'strict': true}),
+        sourceFile: resolveAppScriptPath(rootDir, name, { strict: true }),
         config: componentConfig[name]
       };
     });
@@ -547,7 +557,7 @@ function isPreferredExtension (filename) {
   return (ext in includeExtensions) && !(ext in getExcludedExtensions());
 }
 
-function fixFileExtension (filepath, files, onlyScriptsExportingFunction) {
+function fixFileExtension(filepath, files, onlyScriptsExportingFunction) {
   var results = [];
   var otherFile;
 
@@ -578,10 +588,10 @@ function fixFileExtension (filepath, files, onlyScriptsExportingFunction) {
   return (results.length > 0 ? results[0] : undefined);
 }
 
-function resolveAppScriptPath (rootDir, relativePath, resolveOptions) {
-  var resolvedPath = resolveAppPath (rootDir, relativePath, resolveOptions);
+function resolveAppScriptPath(rootDir, relativePath, resolveOptions) {
+  var resolvedPath = resolveAppPath(rootDir, relativePath, resolveOptions);
   var sourceDir = path.dirname(resolvedPath);
   var files = tryReadDir(sourceDir);
-  var fixedFile = fixFileExtension (resolvedPath, files, false);
+  var fixedFile = fixFileExtension(resolvedPath, files, false);
   return (fixedFile === undefined ? resolvedPath : fixedFile);
 }