From b7a26c7c8834a605670ff1b101f1fbedf0987ae4 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 25 Apr 2016 14:53:28 -0700 Subject: [PATCH 1/2] Extract duplicate NodeModulesNode run npm command logic. **Bug** While debugging through this code, I noticed a lot of logic for running npm commands is duplicated. **Fix** Extract commmon logic to a helper method that provides setup and teardown automatically. --- .../Product/Nodejs/Project/NodeModulesNode.cs | 141 +++++++----------- 1 file changed, 56 insertions(+), 85 deletions(-) diff --git a/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs b/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs index da3e6298f..d05bc5e8c 100644 --- a/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs +++ b/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs @@ -550,11 +550,11 @@ private bool CheckValidCommandTarget(DependencyNode node) { return true; } - public async System.Threading.Tasks.Task InstallMissingModules() { + private async System.Threading.Tasks.Task RunNpmCommand(Func impl) { DoPreCommandActions(); try { using (var commander = NpmController.CreateNpmCommander()) { - await commander.Install(); + await impl(commander); } } catch (NpmNotFoundException nnfe) { ErrorHelper.ReportNpmNotInstalled(null, nnfe); @@ -563,6 +563,12 @@ public async System.Threading.Tasks.Task InstallMissingModules() { } } + public async System.Threading.Tasks.Task InstallMissingModules() { + await RunNpmCommand(commander => { + return commander.Install(); + }); + } + public async System.Threading.Tasks.Task InstallMissingModule(DependencyNode node) { if (!CheckValidCommandTarget(node)) { return; @@ -581,54 +587,40 @@ public async System.Threading.Tasks.Task InstallMissingModule(DependencyNode nod var package = node.Package; var dep = root.PackageJson.AllDependencies[package.Name]; - DoPreCommandActions(); - try { - using (var commander = NpmController.CreateNpmCommander()) { - if (node.GetPropertiesObject().IsGlobalInstall) { - // I genuinely can't see a way this would ever happen but, just to be on the safe side... - await commander.InstallGlobalPackageByVersionAsync( - package.Name, - null == dep ? "*" : dep.VersionRangeText); - } else { - await commander.InstallPackageByVersionAsync( - package.Name, - null == dep ? "*" : dep.VersionRangeText, - DependencyType.Standard, - false); - } + await RunNpmCommand(async commander => { + if (node.GetPropertiesObject().IsGlobalInstall) { + // I genuinely can't see a way this would ever happen but, just to be on the safe side... + await commander.InstallGlobalPackageByVersionAsync( + package.Name, + null == dep ? "*" : dep.VersionRangeText); + } else { + await commander.InstallPackageByVersionAsync( + package.Name, + null == dep ? "*" : dep.VersionRangeText, + DependencyType.Standard, + false); } - } catch (NpmNotFoundException nnfe) { - ErrorHelper.ReportNpmNotInstalled(null, nnfe); - } finally { - AllowCommands(); - } + }); } internal async System.Threading.Tasks.Task UpdateModules(IList nodes) { - DoPreCommandActions(); - try { - using (var commander = NpmController.CreateNpmCommander()) { - if (nodes.Count == 1 && nodes[0] == this) { - await commander.UpdatePackagesAsync(); - } else { - var valid = nodes.OfType().Where(CheckValidCommandTarget).ToList(); - - var list = valid.Where(node => node.GetPropertiesObject().IsGlobalInstall).Select(node => node.Package).ToList(); - if (list.Count > 0) { - await commander.UpdateGlobalPackagesAsync(list); - } + await RunNpmCommand(async commander => { + if (nodes.Count == 1 && nodes[0] == this) { + await commander.UpdatePackagesAsync(); + } else { + var valid = nodes.OfType().Where(CheckValidCommandTarget).ToList(); + + var list = valid.Where(node => node.GetPropertiesObject().IsGlobalInstall).Select(node => node.Package).ToList(); + if (list.Count > 0) { + await commander.UpdateGlobalPackagesAsync(list); + } - list = valid.Where(node => !node.GetPropertiesObject().IsGlobalInstall).Select(node => node.Package).ToList(); - if (list.Count > 0) { - await commander.UpdatePackagesAsync(list); - } + list = valid.Where(node => !node.GetPropertiesObject().IsGlobalInstall).Select(node => node.Package).ToList(); + if (list.Count > 0) { + await commander.UpdatePackagesAsync(list); } } - } catch (NpmNotFoundException nnfe) { - ErrorHelper.ReportNpmNotInstalled(null, nnfe); - } finally { - AllowCommands(); - } + }); } public void UpdateModules() { @@ -639,60 +631,39 @@ public async System.Threading.Tasks.Task UpdateModule(DependencyNode node) { if (!CheckValidCommandTarget(node)) { return; } - DoPreCommandActions(); - try { - using (var commander = NpmController.CreateNpmCommander()) { - if (node.GetPropertiesObject().IsGlobalInstall) { - await commander.UpdateGlobalPackagesAsync(new[] { node.Package }); - } else { - await commander.UpdatePackagesAsync(new[] { node.Package }); - } + await RunNpmCommand(commander => { + if (node.GetPropertiesObject().IsGlobalInstall) { + return commander.UpdateGlobalPackagesAsync(new[] { node.Package }); + } else { + return commander.UpdatePackagesAsync(new[] { node.Package }); } - } catch (NpmNotFoundException nnfe) { - ErrorHelper.ReportNpmNotInstalled(null, nnfe); - } finally { - AllowCommands(); - } + }); } - public async System.Threading.Tasks.Task UninstallModules() { - DoPreCommandActions(); - try { - var selected = _projectNode.GetSelectedNodes(); - using (var commander = NpmController.CreateNpmCommander()) { - foreach (var node in selected.OfType().Where(CheckValidCommandTarget)) { - if (node.GetPropertiesObject().IsGlobalInstall) { - await commander.UninstallGlobalPackageAsync(node.Package.Name); - } else { - await commander.UninstallPackageAsync(node.Package.Name); - } + public System.Threading.Tasks.Task UninstallModules() { + var selected = _projectNode.GetSelectedNodes(); + return RunNpmCommand(async commander => { + foreach (var node in selected.OfType().Where(CheckValidCommandTarget)) { + if (node.GetPropertiesObject().IsGlobalInstall) { + await commander.UninstallGlobalPackageAsync(node.Package.Name); + } else { + await commander.UninstallPackageAsync(node.Package.Name); } } - } catch (NpmNotFoundException nnfe) { - ErrorHelper.ReportNpmNotInstalled(null, nnfe); - } finally { - AllowCommands(); - } + }); } public async System.Threading.Tasks.Task UninstallModule(DependencyNode node) { if (!CheckValidCommandTarget(node)) { return; } - DoPreCommandActions(); - try { - using (var commander = NpmController.CreateNpmCommander()) { - if (node.GetPropertiesObject().IsGlobalInstall) { - await commander.UninstallGlobalPackageAsync(node.Package.Name); - } else { - await commander.UninstallPackageAsync(node.Package.Name); - } + await RunNpmCommand(async commander => { + if (node.GetPropertiesObject().IsGlobalInstall) { + await commander.UninstallGlobalPackageAsync(node.Package.Name); + } else { + await commander.UninstallPackageAsync(node.Package.Name); } - } catch (NpmNotFoundException nnfe) { - ErrorHelper.ReportNpmNotInstalled(null, nnfe); - } finally { - AllowCommands(); - } + }); } #endregion From 516401a5a545ef79a855d62bb479a39a68175034 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 25 Apr 2016 14:57:26 -0700 Subject: [PATCH 2/2] Remove some async usage --- Nodejs/Product/Nodejs/Project/NodeModulesNode.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs b/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs index d05bc5e8c..af4c3c3ca 100644 --- a/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs +++ b/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs @@ -563,10 +563,8 @@ private async System.Threading.Tasks.Task RunNpmCommand(Func { - return commander.Install(); - }); + public System.Threading.Tasks.Task InstallMissingModules() { + return RunNpmCommand(commander => commander.Install()); } public async System.Threading.Tasks.Task InstallMissingModule(DependencyNode node) { @@ -603,8 +601,8 @@ await commander.InstallPackageByVersionAsync( }); } - internal async System.Threading.Tasks.Task UpdateModules(IList nodes) { - await RunNpmCommand(async commander => { + internal System.Threading.Tasks.Task UpdateModules(IList nodes) { + return RunNpmCommand(async commander => { if (nodes.Count == 1 && nodes[0] == this) { await commander.UpdatePackagesAsync(); } else {