Merge pull request #529 from mousetraps/i521

#511, #521 npm tree fixes
microsoft · Oct 14, 2015 · 9ef6b82 · 9ef6b82
2 parents f0d68f9 + 52b9c70
commit 9ef6b82
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 11 deletions.
diff --git a/Nodejs/Product/Npm/SPI/AbstractNodeModules.cs b/Nodejs/Product/Npm/SPI/AbstractNodeModules.cs
@@ -24,7 +24,7 @@ internal abstract class AbstractNodeModules : INodeModules {
         private readonly IDictionary<string, IPackage> _packagesByName = new Dictionary<string, IPackage>();
 
         protected virtual void AddModule(IPackage package) {
-            if (!_packagesSorted.Contains(package) && package.Name != null) {
+            if (package.Name != null && !_packagesByName.ContainsKey(package.Name)) {
                 _packagesSorted.Add(package);
                 _packagesByName[package.Name] = package;
             }

diff --git a/Nodejs/Product/Npm/SPI/NodeModules.cs b/Nodejs/Product/Npm/SPI/NodeModules.cs
@@ -75,10 +75,14 @@ internal class NodeModules : AbstractNodeModules {
             }
 
             if (modulesBase.Length < NativeMethods.MAX_FOLDER_PATH && parent.HasPackageJson) {
-                // Iterate through all dependencies in package.json
-                foreach (var dependency in parent.PackageJson.AllDependencies) {
+                // Iterate through all dependencies in the root package.json
+                // Otherwise, only iterate through "dependencies" because iterating through optional, bundle, etc. dependencies
+                // becomes unmanageable when they are already installed at the root of the project, and the performance impact
+                // typically isn't worth the value add. 
+                var dependencies = depth == 0 ? parent.PackageJson.AllDependencies : parent.PackageJson.Dependencies;
+                foreach (var dependency in dependencies) {
                     var moduleDir = modulesBase;
-                    
+
                     // try to find folder by recursing up tree
                     do {
                         moduleDir = Path.Combine(moduleDir, dependency.Name);
@@ -127,16 +131,25 @@ internal class NodeModules : AbstractNodeModules {
                 return false;
             }
 
-            if (moduleInfo.RequiredBy.Contains(parent.Name)) {
-                return true;
+            IPackage package = moduleInfo.Package;
+
+            if (package == null || depth == 1 || !moduleInfo.RequiredBy.Contains(parent.Path)) {
+                // Create a dummy value for the current package to prevent infinite loops
+                moduleInfo.Package = new PackageProxy();
+
+                moduleInfo.RequiredBy.Add(parent.Path);
+
+                var pkg = new Package(parent, moduleDir, showMissingDevOptionalSubPackages, _allModules, depth);
+                if (dependency != null) {
+                    pkg.RequestedVersionRange = dependency.VersionRangeText;
+                }
+
+                package = moduleInfo.Package = pkg;
             }
 
-            moduleInfo.RequiredBy.Add(parent.Name);
-            var package = new Package(parent, moduleDir, showMissingDevOptionalSubPackages, _allModules, depth);
-            if (dependency != null) {
-                package.RequestedVersionRange = dependency.VersionRangeText;
+            if (parent as IPackage == null || !package.IsMissing || showMissingDevOptionalSubPackages) {
+                AddModule(package);
             }
-            AddModule(package);
 
             return true;
         }
@@ -158,6 +171,9 @@ internal class NodeModules : AbstractNodeModules {
 
     internal class ModuleInfo {
         public int Depth { get; set; }
+
+        public IPackage Package { get; set; }
+
         public IList<string> RequiredBy { get; set; }
 
         internal ModuleInfo(int depth) {