Fix getting production dependencies code (#2789)

* Fix getting production dependencies code

CLI has logic to find which are the "production" dependencies, i.e. which should be copied from `node_modules` to `platforms` dir.
However, when the project has a lot of dependencies (more than 15), on some machines the code leads to error: "Maximum callstack size exceeded". On other machines the code tooks significant time to execute.
After investigation, it turned out the recursion inside `node-modules-dependencies-builder` is incorrect and it adds each package many times to the result array.
Fix the recursion and change the class NodeModulesDependenciesBuilder to be stateless - instead of using properties in `this` object when calculating the production dependencies, the methods will persist the results through the passed args.
This way the whole class can be safely added to `$injector` and used whenever we need the production dependencies. Each time the calculation is started from the beginning, which is the requirement for long living process, where the project may change.
Fix the type of the result, which leads to fix in several other services, where the result has been expected as `IDictionary<smth>`. However it's never been dictionary, it's always been an array. The code, that expected dictionary has been working because the `_.values` method of lodash (used all over the places where the incorrect type of data has been expected), returns the same array when the passed argument is array.
Fix the tests that incorrectly expected dictionary with keys "0", "1", "2", etc.
Remove the usage of Node.js's `fs` module from `NodeModulesDependenciesBuilder` - replace it with `$fs` which allows easir writing of tests.
Require the `nodeModulesDependenciesBuilder` in bootstrap, so it can be correctly resolved by `$injector`.

Add unit tests for `nodeModulesDependenciesBuilder`.

* Use breadth-first search for getting production dependencies

Replace the recursion with breadth-first search algorithm in order to make the code easier for understanding and debugging.
Fix some incorrect code in the tests.

* Add checks before adding new elements to queue of production dependencies

Add check before adding new elements to queue of production dependencies - do not add elements, which are already added and do not read package.json of elements that are already added to "resolvedModules".

Author: rosen-vladimirov

lib/bootstrap.ts

@@ -135,3 +135,5 @@ $injector.requireCommand("extension|*list", "./commands/extensibility/list-exten
 $injector.requireCommand("extension|install", "./commands/extensibility/install-extension");
 $injector.requireCommand("extension|uninstall", "./commands/extensibility/uninstall-extension");
 $injector.requirePublic("extensibilityService", "./services/extensibility-service");
+
+$injector.require("nodeModulesDependenciesBuilder", "./tools/node-modules/node-modules-dependencies-builder");

lib/common

@@ -202,12 +202,36 @@ interface INpmInstallOptions {
 	dependencyType?: string;
 }
 
+/**
+ * Describes npm package installed in node_modules.
+ */
 interface IDependencyData {
+	/**
+	 * The name of the package.
+	 */
 	name: string;
-	version: string;
-	nativescript: any;
-	dependencies?: IStringDictionary;
-	devDependencies?: IStringDictionary;
+
+	/**
+	 * The full path where the package is installed.
+	 */
+	directory: string;
+
+	/**
+	 * The depth inside node_modules dir, where the package is located.
+	 * The <project_dir>/node_modules/ is level 0.
+	 * Level 1 is <project dir>/node_modules/<package name>/node_modules, etc.
+	 */
+	depth: number;
+
+	/**
+	 * Describes the `nativescript` key in package.json of a dependency.
+	 */
+	nativescript?: any;
+
+	/**
+	 * Dependencies of the current module.
+	 */
+	dependencies?: string[];
 }
 
 interface IStaticConfig extends Config.IStaticConfig { }

lib/declarations.d.ts

@@ -275,7 +275,7 @@ interface INodeModulesBuilder {
 }
 
 interface INodeModulesDependenciesBuilder {
-	getProductionDependencies(projectPath: string): void;
+	getProductionDependencies(projectPath: string): IDependencyData[];
 }
 
 interface IBuildInfo {

lib/definitions/platform.d.ts

@@ -225,7 +225,7 @@ interface IPlatformProjectService extends NodeJS.EventEmitter {
 	removePluginNativeCode(pluginData: IPluginData, projectData: IProjectData): Promise<void>;
 
 	afterPrepareAllPlugins(projectData: IProjectData): Promise<void>;
-	beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDictionary<IDependencyData>): Promise<void>;
+	beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): Promise<void>;
 
 	/**
 	 * Gets the path wheren App_Resources should be copied.

lib/definitions/project.d.ts

@@ -400,7 +400,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
 		return;
 	}
 
-	public async beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDictionary<IDependencyData>): Promise<void> {
+	public async beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): Promise<void> {
 		if (!this.$config.debugLivesync) {
 			if (dependencies) {
 				let platformDir = path.join(projectData.platformsDir, "android");

lib/services/android-project-service.ts

@@ -1,7 +1,6 @@
 import * as constants from "../../constants";
 import * as helpers from "../../common/helpers";
 import * as path from "path";
-import { NodeModulesDependenciesBuilder } from "../../tools/node-modules/node-modules-dependencies-builder";
 
 let choki = require("chokidar");
 
@@ -17,7 +16,8 @@ class LiveSyncService implements ILiveSyncService {
 		private $logger: ILogger,
 		private $dispatcher: IFutureDispatcher,
 		private $hooksService: IHooksService,
-		private $processService: IProcessService) { }
+		private $processService: IProcessService,
+		private $nodeModulesDependenciesBuilder: INodeModulesDependenciesBuilder) { }
 
 	public get isInitialized(): boolean { // This function is used from https://github.com/NativeScript/nativescript-dev-typescript/blob/master/lib/before-prepare.js#L4
 		return this._isInitialized;
@@ -94,8 +94,7 @@ class LiveSyncService implements ILiveSyncService {
 
 	private partialSync(syncWorkingDirectory: string, onChangedActions: ((event: string, filePath: string, dispatcher: IFutureDispatcher) => Promise<void>)[], projectData: IProjectData): void {
 		let that = this;
-		let dependenciesBuilder = this.$injector.resolve(NodeModulesDependenciesBuilder, {});
-		let productionDependencies = dependenciesBuilder.getProductionDependencies(projectData.projectDir);
+		let productionDependencies = this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir);
 		let pattern = ["app"];
 
 		if (this.$options.syncAllFiles) {

lib/services/livesync/livesync-service.ts

@@ -1,11 +1,11 @@
 import * as shelljs from "shelljs";
 import { TnsModulesCopy, NpmPluginPrepare } from "./node-modules-dest-copy";
-import { NodeModulesDependenciesBuilder } from "./node-modules-dependencies-builder";
 
 export class NodeModulesBuilder implements INodeModulesBuilder {
 	constructor(private $fs: IFileSystem,
 		private $injector: IInjector,
-		private $options: IOptions
+		private $options: IOptions,
+		private $nodeModulesDependenciesBuilder: INodeModulesDependenciesBuilder
 	) { }
 
 	public async prepareNodeModules(absoluteOutputPath: string, platform: string, lastModifiedTime: Date, projectData: IProjectData): Promise<void> {
@@ -14,8 +14,7 @@ export class NodeModulesBuilder implements INodeModulesBuilder {
 			lastModifiedTime = null;
 		}
 
-		let dependenciesBuilder = this.$injector.resolve(NodeModulesDependenciesBuilder, {});
-		let productionDependencies = dependenciesBuilder.getProductionDependencies(projectData.projectDir);
+		let productionDependencies = this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir);
 
 		if (!this.$options.bundle) {
 			const tnsModulesCopy = this.$injector.resolve(TnsModulesCopy, {

lib/tools/node-modules/node-modules-builder.ts

@@ -1,111 +1,110 @@
 import * as path from "path";
-import * as fs from "fs";
+import { NODE_MODULES_FOLDER_NAME, PACKAGE_JSON_FILE_NAME } from "../../constants";
 
-export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesBuilder {
-	private projectPath: string;
-	private rootNodeModulesPath: string;
-	private resolvedDependencies: any[];
-	private seen: any;
-
-	public constructor(private $fs: IFileSystem) {
-		this.seen = {};
-		this.resolvedDependencies = [];
-	}
-
-	public getProductionDependencies(projectPath: string): any[] {
-		this.projectPath = projectPath;
-		this.rootNodeModulesPath = path.join(this.projectPath, "node_modules");
-
-		let projectPackageJsonpath = path.join(this.projectPath, "package.json");
-		let packageJsonContent = this.$fs.readJson(projectPackageJsonpath);
-
-		_.keys(packageJsonContent.dependencies).forEach(dependencyName => {
-			let depth = 0;
-			let directory = path.join(this.rootNodeModulesPath, dependencyName);
-
-			// find and traverse child with name `key`, parent's directory -> dep.directory
-			this.traverseDependency(dependencyName, directory, depth);
-		});
-
-		return this.resolvedDependencies;
-	}
+interface IDependencyDescription {
+	parentDir: string;
+	name: string;
+	depth: number;
+}
 
-	private traverseDependency(name: string, currentModulePath: string, depth: number): void {
-		// Check if child has been extracted in the parent's node modules, AND THEN in `node_modules`
-		// Slower, but prevents copying wrong versions if multiple of the same module are installed
-		// Will also prevent copying project's devDependency's version if current module depends on another version
-		let modulePath = path.join(currentModulePath, "node_modules", name); // node_modules/parent/node_modules/<package>
-		let alternativeModulePath = path.join(this.rootNodeModulesPath, name);
+export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesBuilder {
+	public constructor(private $fs: IFileSystem) { }
+
+	public getProductionDependencies(projectPath: string): IDependencyData[] {
+		const rootNodeModulesPath = path.join(projectPath, NODE_MODULES_FOLDER_NAME);
+		const projectPackageJsonPath = path.join(projectPath, PACKAGE_JSON_FILE_NAME);
+		const packageJsonContent = this.$fs.readJson(projectPackageJsonPath);
+		const dependencies = packageJsonContent && packageJsonContent.dependencies;
+
+		let resolvedDependencies: IDependencyData[] = [];
+
+		let queue: IDependencyDescription[] = _.keys(dependencies)
+			.map(dependencyName => ({
+				parentDir: projectPath,
+				name: dependencyName,
+				depth: 0
+			}));
+
+		while (queue.length) {
+			const currentModule = queue.shift();
+			const resolvedDependency = this.findModule(rootNodeModulesPath, currentModule.parentDir, currentModule.name, currentModule.depth, resolvedDependencies);
+
+			if (resolvedDependency && !_.some(resolvedDependencies, r => r.directory === resolvedDependency.directory)) {
+				_.each(resolvedDependency.dependencies, d => {
+					const dependency: IDependencyDescription = { name: d, parentDir: resolvedDependency.directory, depth: resolvedDependency.depth + 1 };
+
+					const shouldAdd = !_.some(queue, element =>
+						element.name === dependency.name &&
+						element.parentDir === dependency.parentDir &&
+						element.depth === dependency.depth);
+
+					if (shouldAdd) {
+						queue.push(dependency);
+					}
+				});
+
+				resolvedDependencies.push(resolvedDependency);
+			}
+		}
 
-		this.findModule(modulePath, alternativeModulePath, name, depth);
+		return resolvedDependencies;
 	}
 
-	private findModule(modulePath: string, alternativeModulePath: string, name: string, depth: number) {
-		let exists = this.moduleExists(modulePath);
+	private findModule(rootNodeModulesPath: string, parentModulePath: string, name: string, depth: number, resolvedDependencies: IDependencyData[]): IDependencyData {
+		let modulePath = path.join(parentModulePath, NODE_MODULES_FOLDER_NAME, name); // node_modules/parent/node_modules/<package>
+		const rootModulesPath = path.join(rootNodeModulesPath, name);
+		let depthInNodeModules = depth;
 
-		if (exists) {
-			if (this.seen[modulePath]) {
-				return;
+		if (!this.moduleExists(modulePath)) {
+			modulePath = rootModulesPath; // /node_modules/<package>
+			if (!this.moduleExists(modulePath)) {
+				return null;
 			}
 
-			let dependency = this.addDependency(name, modulePath, depth + 1);
-			this.readModuleDependencies(modulePath, depth + 1, dependency);
-		} else {
-			modulePath = alternativeModulePath; // /node_modules/<package>
-			exists = this.moduleExists(modulePath);
+			depthInNodeModules = 0;
+		}
 
-			if (!exists) {
-				return;
-			}
+		if (_.some(resolvedDependencies, r => r.name === name && r.directory === modulePath)) {
+			return null;
 
-			if (this.seen[modulePath]) {
-				return;
-			}
-
-			let dependency = this.addDependency(name, modulePath, 0);
-			this.readModuleDependencies(modulePath, 0, dependency);
 		}
 
-		this.seen[modulePath] = true;
+		return this.getDependencyData(name, modulePath, depthInNodeModules);
 	}
 
-	private readModuleDependencies(modulePath: string, depth: number, currentModule: any): void {
-		let packageJsonPath = path.join(modulePath, 'package.json');
-		let packageJsonExists = fs.lstatSync(packageJsonPath).isFile();
+	private getDependencyData(name: string, directory: string, depth: number): IDependencyData {
+		const dependency: IDependencyData = {
+			name,
+			directory,
+			depth
+		};
+
+		const packageJsonPath = path.join(directory, PACKAGE_JSON_FILE_NAME);
+		const packageJsonExists = this.$fs.getLsStats(packageJsonPath).isFile();
 
 		if (packageJsonExists) {
 			let packageJsonContents = this.$fs.readJson(packageJsonPath);
 
 			if (!!packageJsonContents.nativescript) {
 				// add `nativescript` property, necessary for resolving plugins
-				currentModule.nativescript = packageJsonContents.nativescript;
+				dependency.nativescript = packageJsonContents.nativescript;
 			}
 
-			_.keys(packageJsonContents.dependencies).forEach((dependencyName) => {
-				this.traverseDependency(dependencyName, modulePath, depth);
-			});
+			dependency.dependencies = _.keys(packageJsonContents.dependencies);
+			return dependency;
 		}
-	}
 
-	private addDependency(name: string, directory: string, depth: number): any {
-		let dependency: any = {
-			name,
-			directory,
-			depth
-		};
-
-		this.resolvedDependencies.push(dependency);
-
-		return dependency;
+		return null;
 	}
 
 	private moduleExists(modulePath: string): boolean {
 		try {
-			let exists = fs.lstatSync(modulePath);
-			if (exists.isSymbolicLink()) {
-				exists = fs.lstatSync(fs.realpathSync(modulePath));
+			let modulePathLsStat = this.$fs.getLsStats(modulePath);
+			if (modulePathLsStat.isSymbolicLink()) {
+				modulePathLsStat = this.$fs.getLsStats(this.$fs.realpath(modulePath));
 			}
-			return exists.isDirectory();
+
+			return modulePathLsStat.isDirectory();
 		} catch (e) {
 			return false;
 		}

lib/tools/node-modules/node-modules-dependencies-builder.ts

@@ -15,7 +15,7 @@ export class TnsModulesCopy {
 	) {
 	}
 
-	public copyModules(dependencies: any[], platform: string): void {
+	public copyModules(dependencies: IDependencyData[], platform: string): void {
 		for (let entry in dependencies) {
 			let dependency = dependencies[entry];
 
@@ -34,7 +34,7 @@ export class TnsModulesCopy {
 		}
 	}
 
-	private copyDependencyDir(dependency: any): void {
+	private copyDependencyDir(dependency: IDependencyData): void {
 		if (dependency.depth === 0) {
 			let isScoped = dependency.name.indexOf("@") === 0;
 			let targetDir = this.outputRoot;
@@ -61,18 +61,18 @@ export class NpmPluginPrepare {
 	) {
 	}
 
-	protected async beforePrepare(dependencies: IDictionary<IDependencyData>, platform: string, projectData: IProjectData): Promise<void> {
+	protected async beforePrepare(dependencies: IDependencyData[], platform: string, projectData: IProjectData): Promise<void> {
 		await this.$platformsData.getPlatformData(platform, projectData).platformProjectService.beforePrepareAllPlugins(projectData, dependencies);
 	}
 
-	protected async afterPrepare(dependencies: IDictionary<IDependencyData>, platform: string, projectData: IProjectData): Promise<void> {
+	protected async afterPrepare(dependencies: IDependencyData[], platform: string, projectData: IProjectData): Promise<void> {
 		await this.$platformsData.getPlatformData(platform, projectData).platformProjectService.afterPrepareAllPlugins(projectData);
 		this.writePreparedDependencyInfo(dependencies, platform, projectData);
 	}
 
-	private writePreparedDependencyInfo(dependencies: IDictionary<IDependencyData>, platform: string, projectData: IProjectData): void {
+	private writePreparedDependencyInfo(dependencies: IDependencyData[], platform: string, projectData: IProjectData): void {
 		let prepareData: IDictionary<boolean> = {};
-		_.values(dependencies).forEach(d => {
+		_.each(dependencies, d => {
 			prepareData[d.name] = true;
 		});
 		this.$fs.createDirectory(this.preparedPlatformsDir(platform, projectData));
@@ -101,18 +101,18 @@ export class NpmPluginPrepare {
 		return this.$fs.readJson(this.preparedPlatformsFile(platform, projectData), "utf8");
 	}
 
-	private allPrepared(dependencies: IDictionary<IDependencyData>, platform: string, projectData: IProjectData): boolean {
+	private allPrepared(dependencies: IDependencyData[], platform: string, projectData: IProjectData): boolean {
 		let result = true;
 		const previouslyPrepared = this.getPreviouslyPreparedDependencies(platform, projectData);
-		_.values(dependencies).forEach(d => {
+		_.each(dependencies, d => {
 			if (!previouslyPrepared[d.name]) {
 				result = false;
 			}
 		});
 		return result;
 	}
 
-	public async preparePlugins(dependencies: IDictionary<IDependencyData>, platform: string, projectData: IProjectData): Promise<void> {
+	public async preparePlugins(dependencies: IDependencyData[], platform: string, projectData: IProjectData): Promise<void> {
 		if (_.isEmpty(dependencies) || this.allPrepared(dependencies, platform, projectData)) {
 			return;
 		}