From 0802444cc49f32997dece39925690c2e4ea1e401 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Sun, 7 May 2017 19:29:22 +0300 Subject: [PATCH 1/3] 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`. 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`. --- lib/bootstrap.ts | 2 + lib/common | 2 +- lib/declarations.d.ts | 27 +- lib/definitions/platform.d.ts | 2 +- lib/definitions/project.d.ts | 2 +- lib/services/android-project-service.ts | 2 +- lib/services/livesync/livesync-service.ts | 7 +- .../node-modules/node-modules-builder.ts | 7 +- .../node-modules-dependencies-builder.ts | 106 +++--- .../node-modules/node-modules-dest-copy.ts | 18 +- test/npm-support.ts | 4 +- test/plugin-prepare.ts | 31 +- test/plugins-service.ts | 15 +- test/stubs.ts | 4 + .../node-modules-dependencies-builder.ts | 318 ++++++++++++++++++ 15 files changed, 440 insertions(+), 107 deletions(-) create mode 100644 test/tools/node-modules/node-modules-dependencies-builder.ts diff --git a/lib/bootstrap.ts b/lib/bootstrap.ts index 25ec914ba8..d2d81c8b4c 100644 --- a/lib/bootstrap.ts +++ b/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"); diff --git a/lib/common b/lib/common index 6e4c95ebbf..e66bb06aed 160000 --- a/lib/common +++ b/lib/common @@ -1 +1 @@ -Subproject commit 6e4c95ebbfbf118c463f1e09f39505d5ceb8e48c +Subproject commit e66bb06aedefe4f493edecc4223d4a9c4d97564c diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index 5fb04bf394..8760c4d2ec 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -202,12 +202,31 @@ interface INpmInstallOptions { dependencyType?: string; } +/** + * Describes a package installed in node_modules directory of a project. + */ 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 /node_modules/ is level 0. + * Level 1 is /node_modules//node_modules, etc. + */ + depth: number; + + /** + * Describes the `nativescript` key in package.json of a dependency. + */ + nativescript?: any; } interface IStaticConfig extends Config.IStaticConfig { } diff --git a/lib/definitions/platform.d.ts b/lib/definitions/platform.d.ts index d93d820770..8c6a7c5d7d 100644 --- a/lib/definitions/platform.d.ts +++ b/lib/definitions/platform.d.ts @@ -275,7 +275,7 @@ interface INodeModulesBuilder { } interface INodeModulesDependenciesBuilder { - getProductionDependencies(projectPath: string): void; + getProductionDependencies(projectPath: string): IDependencyData[]; } interface IBuildInfo { diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index 50c2e58acc..09e9293c6c 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -225,7 +225,7 @@ interface IPlatformProjectService extends NodeJS.EventEmitter { removePluginNativeCode(pluginData: IPluginData, projectData: IProjectData): Promise; afterPrepareAllPlugins(projectData: IProjectData): Promise; - beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDictionary): Promise; + beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): Promise; /** * Gets the path wheren App_Resources should be copied. diff --git a/lib/services/android-project-service.ts b/lib/services/android-project-service.ts index a45f50f2a1..e2804185e2 100644 --- a/lib/services/android-project-service.ts +++ b/lib/services/android-project-service.ts @@ -400,7 +400,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject return; } - public async beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDictionary): Promise { + public async beforePrepareAllPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): Promise { if (!this.$config.debugLivesync) { if (dependencies) { let platformDir = path.join(projectData.platformsDir, "android"); diff --git a/lib/services/livesync/livesync-service.ts b/lib/services/livesync/livesync-service.ts index 3e11bf2708..5f57d8dcc8 100644 --- a/lib/services/livesync/livesync-service.ts +++ b/lib/services/livesync/livesync-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)[], 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) { diff --git a/lib/tools/node-modules/node-modules-builder.ts b/lib/tools/node-modules/node-modules-builder.ts index 52bea13107..ac1556151d 100644 --- a/lib/tools/node-modules/node-modules-builder.ts +++ b/lib/tools/node-modules/node-modules-builder.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 { @@ -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, { diff --git a/lib/tools/node-modules/node-modules-dependencies-builder.ts b/lib/tools/node-modules/node-modules-dependencies-builder.ts index eb946c30ce..6011743e2b 100644 --- a/lib/tools/node-modules/node-modules-dependencies-builder.ts +++ b/lib/tools/node-modules/node-modules-dependencies-builder.ts @@ -1,77 +1,63 @@ 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 constructor(private $fs: IFileSystem) { } - public getProductionDependencies(projectPath: string): any[] { - this.projectPath = projectPath; - this.rootNodeModulesPath = path.join(this.projectPath, "node_modules"); + 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 projectPackageJsonpath = path.join(this.projectPath, "package.json"); - let packageJsonContent = this.$fs.readJson(projectPackageJsonpath); + let resolvedDependencies: IDependencyData[] = []; - _.keys(packageJsonContent.dependencies).forEach(dependencyName => { - let depth = 0; - let directory = path.join(this.rootNodeModulesPath, dependencyName); + _.keys(dependencies) + .forEach(dependencyName => { + const depth = 0; + const directory = path.join(rootNodeModulesPath, dependencyName); - // find and traverse child with name `key`, parent's directory -> dep.directory - this.traverseDependency(dependencyName, directory, depth); - }); + // find and traverse child with name `key`, parent's directory -> dep.directory + this.traverseDependency(resolvedDependencies, rootNodeModulesPath, dependencyName, directory, depth); + }); - return this.resolvedDependencies; + return resolvedDependencies; } - private traverseDependency(name: string, currentModulePath: string, depth: number): void { + private traverseDependency(resolvedDependencies: IDependencyData[], rootNodeModulesPath: string, 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/ - let alternativeModulePath = path.join(this.rootNodeModulesPath, name); + const modulePath = path.join(currentModulePath, NODE_MODULES_FOLDER_NAME, name); // node_modules/parent/node_modules/ + const alternativeModulePath = path.join(rootNodeModulesPath, name); - this.findModule(modulePath, alternativeModulePath, name, depth); + this.findModule(resolvedDependencies, rootNodeModulesPath, modulePath, alternativeModulePath, name, depth); } - private findModule(modulePath: string, alternativeModulePath: string, name: string, depth: number) { + private findModule(resolvedDependencies: IDependencyData[], rootNodeModulesPath: string, modulePath: string, rootModulesPath: string, name: string, depth: number): void { let exists = this.moduleExists(modulePath); + let depthInNodeModules = depth + 1; - if (exists) { - if (this.seen[modulePath]) { - return; - } - - let dependency = this.addDependency(name, modulePath, depth + 1); - this.readModuleDependencies(modulePath, depth + 1, dependency); - } else { - modulePath = alternativeModulePath; // /node_modules/ + if (!exists) { + modulePath = rootModulesPath; // /node_modules/ exists = this.moduleExists(modulePath); - if (!exists) { - return; - } - - if (this.seen[modulePath]) { - return; - } + depthInNodeModules = 0; + } - let dependency = this.addDependency(name, modulePath, 0); - this.readModuleDependencies(modulePath, 0, dependency); + if (!exists || _.some(resolvedDependencies, deps => deps.directory === modulePath)) { + return; } - this.seen[modulePath] = true; + const dependency = this.getDependencyInfo(name, modulePath, depthInNodeModules); + resolvedDependencies.push(dependency); + + this.readModuleDependencies(resolvedDependencies, rootNodeModulesPath, modulePath, depthInNodeModules, dependency); } - private readModuleDependencies(modulePath: string, depth: number, currentModule: any): void { - let packageJsonPath = path.join(modulePath, 'package.json'); - let packageJsonExists = fs.lstatSync(packageJsonPath).isFile(); + private readModuleDependencies(resolvedDependencies: IDependencyData[], rootNodeModulesPath: string, modulePath: string, depth: number, currentModule: IDependencyData): void { + const packageJsonPath = path.join(modulePath, PACKAGE_JSON_FILE_NAME); + const packageJsonExists = this.$fs.getLsStats(packageJsonPath).isFile(); if (packageJsonExists) { let packageJsonContents = this.$fs.readJson(packageJsonPath); @@ -81,31 +67,31 @@ export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesB currentModule.nativescript = packageJsonContents.nativescript; } - _.keys(packageJsonContents.dependencies).forEach((dependencyName) => { - this.traverseDependency(dependencyName, modulePath, depth); - }); + _.keys(packageJsonContents.dependencies) + .forEach((dependencyName) => { + this.traverseDependency(resolvedDependencies, rootNodeModulesPath, dependencyName, modulePath, depth); + }); } } - private addDependency(name: string, directory: string, depth: number): any { - let dependency: any = { + private getDependencyInfo(name: string, directory: string, depth: number): IDependencyData { + const dependency: IDependencyData = { name, directory, depth }; - this.resolvedDependencies.push(dependency); - return dependency; } 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; } diff --git a/lib/tools/node-modules/node-modules-dest-copy.ts b/lib/tools/node-modules/node-modules-dest-copy.ts index de313c1f66..c5cb4f4864 100644 --- a/lib/tools/node-modules/node-modules-dest-copy.ts +++ b/lib/tools/node-modules/node-modules-dest-copy.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, platform: string, projectData: IProjectData): Promise { + protected async beforePrepare(dependencies: IDependencyData[], platform: string, projectData: IProjectData): Promise { await this.$platformsData.getPlatformData(platform, projectData).platformProjectService.beforePrepareAllPlugins(projectData, dependencies); } - protected async afterPrepare(dependencies: IDictionary, platform: string, projectData: IProjectData): Promise { + protected async afterPrepare(dependencies: IDependencyData[], platform: string, projectData: IProjectData): Promise { await this.$platformsData.getPlatformData(platform, projectData).platformProjectService.afterPrepareAllPlugins(projectData); this.writePreparedDependencyInfo(dependencies, platform, projectData); } - private writePreparedDependencyInfo(dependencies: IDictionary, platform: string, projectData: IProjectData): void { + private writePreparedDependencyInfo(dependencies: IDependencyData[], platform: string, projectData: IProjectData): void { let prepareData: IDictionary = {}; - _.values(dependencies).forEach(d => { + _.each(dependencies, d => { prepareData[d.name] = true; }); this.$fs.createDirectory(this.preparedPlatformsDir(platform, projectData)); @@ -101,10 +101,10 @@ export class NpmPluginPrepare { return this.$fs.readJson(this.preparedPlatformsFile(platform, projectData), "utf8"); } - private allPrepared(dependencies: IDictionary, 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; } @@ -112,7 +112,7 @@ export class NpmPluginPrepare { return result; } - public async preparePlugins(dependencies: IDictionary, platform: string, projectData: IProjectData): Promise { + public async preparePlugins(dependencies: IDependencyData[], platform: string, projectData: IProjectData): Promise { if (_.isEmpty(dependencies) || this.allPrepared(dependencies, platform, projectData)) { return; } diff --git a/test/npm-support.ts b/test/npm-support.ts index 00fe04a5cc..2c030101f8 100644 --- a/test/npm-support.ts +++ b/test/npm-support.ts @@ -27,6 +27,7 @@ import { XmlValidator } from "../lib/xml-validator"; import { LockFile } from "../lib/lockfile"; import ProjectChangesLib = require("../lib/services/project-changes-service"); import { Messages } from "../lib/common/messages/messages"; +import { NodeModulesDependenciesBuilder } from "../lib/tools/node-modules/node-modules-dependencies-builder"; import path = require("path"); import temp = require("temp"); @@ -81,9 +82,10 @@ function createTestInjector(): IInjector { testInjector.register("projectChangesService", ProjectChangesLib.ProjectChangesService); testInjector.register("emulatorPlatformService", stubs.EmulatorPlatformService); testInjector.register("analyticsService", { - track: async () => undefined + track: async (): Promise => undefined }); testInjector.register("messages", Messages); + testInjector.register("nodeModulesDependenciesBuilder", NodeModulesDependenciesBuilder); return testInjector; } diff --git a/test/plugin-prepare.ts b/test/plugin-prepare.ts index 73bcde4131..45f3664147 100644 --- a/test/plugin-prepare.ts +++ b/test/plugin-prepare.ts @@ -14,13 +14,13 @@ class TestNpmPluginPrepare extends NpmPluginPrepare { return this.previouslyPrepared; } - protected async beforePrepare(dependencies: IDictionary, platform: string): Promise { - _.values(dependencies).forEach(d => { + protected async beforePrepare(dependencies: IDependencyData[], platform: string): Promise { + _.each(dependencies, d => { this.preparedDependencies[d.name] = true; }); } - protected async afterPrepare(dependencies: IDictionary, platform: string): Promise { + protected async afterPrepare(dependencies: IDependencyData[], platform: string): Promise { // DO NOTHING } } @@ -28,37 +28,40 @@ class TestNpmPluginPrepare extends NpmPluginPrepare { describe("Plugin preparation", () => { it("skips prepare if no plugins", async () => { const pluginPrepare = new TestNpmPluginPrepare({}); - await pluginPrepare.preparePlugins({}, "android", null); + await pluginPrepare.preparePlugins([], "android", null); assert.deepEqual({}, pluginPrepare.preparedDependencies); }); it("skips prepare if every plugin prepared", async () => { const pluginPrepare = new TestNpmPluginPrepare({ "tns-core-modules-widgets": true }); - const testDependencies: IDictionary = { - "0": { + const testDependencies: IDependencyData[] = [ + { name: "tns-core-modules-widgets", - version: "1.0.0", + depth: 0, + directory: "some dir", nativescript: null, } - }; + ]; await pluginPrepare.preparePlugins(testDependencies, "android", null); assert.deepEqual({}, pluginPrepare.preparedDependencies); }); it("saves prepared plugins after preparation", async () => { const pluginPrepare = new TestNpmPluginPrepare({ "tns-core-modules-widgets": true }); - const testDependencies: IDictionary = { - "0": { + const testDependencies: IDependencyData[] = [ + { name: "tns-core-modules-widgets", - version: "1.0.0", + depth: 0, + directory: "some dir", nativescript: null, }, - "1": { + { name: "nativescript-calendar", - version: "1.0.0", + depth: 0, + directory: "some dir", nativescript: null, } - }; + ]; await pluginPrepare.preparePlugins(testDependencies, "android", null); const prepareData = { "tns-core-modules-widgets": true, "nativescript-calendar": true }; assert.deepEqual(prepareData, pluginPrepare.preparedDependencies); diff --git a/test/plugins-service.ts b/test/plugins-service.ts index cc440a608d..2ad19b077a 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -477,14 +477,15 @@ describe("Plugins service", () => { let pluginName = "mySamplePlugin"; let projectFolder = createProjectFile(testInjector); let pluginFolderPath = path.join(projectFolder, pluginName); - let pluginJsonData = { - "name": pluginName, - "version": "0.0.1", - "nativescript": { - "platforms": { - "android": "0.10.0" + let pluginJsonData: IDependencyData = { + name: pluginName, + nativescript: { + platforms: { + android: "0.10.0" } - } + }, + depth: 0, + directory: "some dir" }; let fs = testInjector.resolve("fs"); fs.writeJson(path.join(pluginFolderPath, "package.json"), pluginJsonData); diff --git a/test/stubs.ts b/test/stubs.ts index 3893ba69f1..eabbb21d7c 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -171,6 +171,10 @@ export class FileSystemStub implements IFileSystem { deleteEmptyParents(directory: string): void { } utimes(path: string, atime: Date, mtime: Date): void { } + + realpath(filePath: string): string { + return null; + } } export class ErrorsStub implements IErrors { diff --git a/test/tools/node-modules/node-modules-dependencies-builder.ts b/test/tools/node-modules/node-modules-dependencies-builder.ts new file mode 100644 index 0000000000..7850839528 --- /dev/null +++ b/test/tools/node-modules/node-modules-dependencies-builder.ts @@ -0,0 +1,318 @@ +import { Yok } from "../../../lib/common/yok"; +import { assert } from "chai"; +import { NodeModulesDependenciesBuilder } from "../../../lib/tools/node-modules/node-modules-dependencies-builder"; +import * as path from "path"; +import * as constants from "../../../lib/constants"; + +interface IDependencyInfo { + name: string; + version: string; + depth: number; + dependencies?: IDependencyInfo[]; + nativescript?: any; +}; + +// TODO: Add integration tests. +// The tests assumes npm 3 or later is used, so all dependencies (and their dependencies) will be installed at the root node_modules +describe("nodeModulesDependenciesBuilder", () => { + const pathToProject = "some path"; + const getTestInjector = (): IInjector => { + const testInjector = new Yok(); + testInjector.register("fs", { + readJson: (pathToFile: string): any => undefined + }); + + return testInjector; + }; + + describe("getProductionDependencies", () => { + describe("returns empty array", () => { + const validateResultIsEmpty = async (resultOfReadJson: any) => { + const testInjector = getTestInjector(); + const fs = testInjector.resolve("fs"); + fs.readJson = (filename: string, encoding?: string): any => { + return resultOfReadJson; + }; + + const nodeModulesDependenciesBuilder = testInjector.resolve(NodeModulesDependenciesBuilder); + const result = await nodeModulesDependenciesBuilder.getProductionDependencies(pathToProject); + + assert.deepEqual(result, []); + }; + + it("when package.json does not have any data", async () => { + await validateResultIsEmpty(null); + }); + + it("when package.json does not have dependencies section", async () => { + await validateResultIsEmpty({ name: "some name", devDependencies: { a: "1.0.0" } }); + }); + }); + + describe("returns correct dependencies", () => { + /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * Helper functions for easier writing of consecutive tests in the suite. * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + + const getPathToDependencyInNodeModules = (dependencyName: string): string => { + return path.join(pathToProject, constants.NODE_MODULES_FOLDER_NAME, dependencyName); + }; + + const getNodeModuleInfoForExpecteDependency = (name: string, depth: number, nativescript?: any): IDependencyData => { + let result: IDependencyData = { + name: path.basename(name), + directory: getPathToDependencyInNodeModules(name), + depth + }; + + if (nativescript) { + result.nativescript = nativescript; + } + + return result; + }; + + const getPathToPackageJsonOfDependency = (dependencyName: string): string => { + return path.join(getPathToDependencyInNodeModules(dependencyName), constants.PACKAGE_JSON_FILE_NAME); + }; + + const getDependenciesObjectFromDependencyInfo = (depInfos: IDependencyInfo[], nativescript: any): { dependencies: any, nativescript?: any } => { + const dependencies: any = {}; + _.each(depInfos, innerDependency => { + dependencies[innerDependency.name] = innerDependency.version; + }); + + let result: any = { + dependencies + }; + + if (nativescript) { + result.nativescript = nativescript; + } + + return result; + }; + + const getDependenciesObject = (filename: string, deps: IDependencyInfo[]): { dependencies: any } => { + let result: { dependencies: any } = null; + for (let dependencyInfo of deps) { + const pathToPackageJson = getPathToPackageJsonOfDependency(dependencyInfo.name); + if (filename === pathToPackageJson) { + return getDependenciesObjectFromDependencyInfo(dependencyInfo.dependencies, dependencyInfo.nativescript); + } + + if (dependencyInfo.dependencies) { + result = getDependenciesObject(filename, dependencyInfo.dependencies); + if (result) { + break; + } + } + } + + return result; + }; + + const generateTest = (rootDeps: IDependencyInfo[]): INodeModulesDependenciesBuilder => { + const testInjector = getTestInjector(); + const nodeModulesDependenciesBuilder = testInjector.resolve(NodeModulesDependenciesBuilder); + const fs = testInjector.resolve("fs"); + + fs.readJson = (filename: string, encoding?: string): any => { + const innerDependency = getDependenciesObject(filename, rootDeps); + return innerDependency || getDependenciesObjectFromDependencyInfo(rootDeps, null); + }; + + const isDirectory = (searchedPath: string, currentRootPath: string, deps: IDependencyInfo[], currentDepthLevel: number): boolean => { + let result = false; + + for (let dependencyInfo of deps) { + const pathToDependency = path.join(currentRootPath, constants.NODE_MODULES_FOLDER_NAME, dependencyInfo.name); + + if (pathToDependency === searchedPath && currentDepthLevel === dependencyInfo.depth) { + return true; + } + + if (dependencyInfo.dependencies) { + result = isDirectory(searchedPath, pathToDependency, dependencyInfo.dependencies, currentDepthLevel + 1); + if (result) { + break; + } + } + } + + return result; + }; + + const isPackageJsonOfDependency = (searchedPath: string, currentRootPath: string, deps: IDependencyInfo[], currentDepthLevel: number): boolean => { + let result = false; + for (let dependencyInfo of deps) { + const pathToDependency = path.join(currentRootPath, constants.NODE_MODULES_FOLDER_NAME, dependencyInfo.name); + + const pathToPackageJson = path.join(pathToDependency, constants.PACKAGE_JSON_FILE_NAME); + + if (pathToPackageJson === searchedPath && currentDepthLevel === dependencyInfo.depth) { + return true; + } + + if (dependencyInfo.dependencies) { + result = isPackageJsonOfDependency(searchedPath, pathToDependency, dependencyInfo.dependencies, currentDepthLevel + 1); + if (result) { + break; + } + } + } + + return result; + }; + + fs.getLsStats = (pathToStat: string): any => { + return { + isDirectory: (): boolean => isDirectory(pathToStat, pathToProject, rootDeps, 0), + isSymbolicLink: (): boolean => false, + isFile: (): boolean => isPackageJsonOfDependency(pathToStat, pathToProject, rootDeps, 0) + }; + }; + + return nodeModulesDependenciesBuilder; + }; + + const generateDependency = (name: string, version: string, depth: number, dependencies: IDependencyInfo[], nativescript?: any): IDependencyInfo => { + return { + name, + version, + depth, + dependencies, + nativescript + }; + }; + + /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * END of helper functions for easier writing of consecutive tests in the suite. * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + + const firstPackage = "firstPackage"; + const secondPackage = "secondPackage"; + const thirdPackage = "thirdPackage"; + + it("when all dependencies are installed at the root level of the project", async () => { + // The test validates the following dependency tree, when npm 3+ is used. + // + // ├── firstPackage@1.0.0 + // ├── secondPackage@1.1.0 + // └── thirdPackage@1.2.0 + + const rootDeps: IDependencyInfo[] = [ + generateDependency(firstPackage, "1.0.0", 0, null), + generateDependency(secondPackage, "1.1.0", 0, null), + generateDependency(thirdPackage, "1.2.0", 0, null) + ]; + + const nodeModulesDependenciesBuilder = generateTest(rootDeps); + const actualResult = await nodeModulesDependenciesBuilder.getProductionDependencies(pathToProject); + + const expectedResult: IDependencyData[] = [ + getNodeModuleInfoForExpecteDependency(firstPackage, 0), + getNodeModuleInfoForExpecteDependency(secondPackage, 0), + getNodeModuleInfoForExpecteDependency(thirdPackage, 0) + ]; + + assert.deepEqual(actualResult, expectedResult); + }); + + it("when the project has a dependency to a package and one of the other packages has dependency to other version of this package", async () => { + // The test validates the following dependency tree, when npm 3+ is used. + // + // ├─┬ firstPackage@1.0.0 + // │ └── secondPackage@1.2.0 + // └── secondPackage@1.1.0 + + const rootDeps: IDependencyInfo[] = [ + generateDependency(firstPackage, "1.0.0", 0, [generateDependency(secondPackage, "1.2.0", 1, null)]), + generateDependency(secondPackage, "1.1.0", 0, null) + ]; + + const expectedResult: IDependencyData[] = [ + getNodeModuleInfoForExpecteDependency(firstPackage, 0), + getNodeModuleInfoForExpecteDependency(path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, secondPackage), 1), + getNodeModuleInfoForExpecteDependency(secondPackage, 0) + ]; + + const nodeModulesDependenciesBuilder = generateTest(rootDeps); + const actualResult = await nodeModulesDependenciesBuilder.getProductionDependencies(pathToProject); + assert.deepEqual(actualResult, expectedResult); + }); + + it("when several package depend on different versions of other packages", async () => { + // The test validates the following dependency tree, when npm 3+ is used. + // + // ├─┬ firstPackage@1.0.0 + // │ ├─┬ secondPackage@1.1.0 + // │ │ └── thirdPackage@1.2.0 + // │ └── thirdPackage@1.1.0 + // ├── secondPackage@1.0.0 + // └── thirdPackage@1.0.0 + + const rootDeps: IDependencyInfo[] = [ + generateDependency(firstPackage, "1.0.0", 0, [ + generateDependency(secondPackage, "1.1.0", 1, [ + generateDependency(thirdPackage, "1.2.0", 2, null) + ]), + generateDependency(thirdPackage, "1.1.0", 1, null) + ]), + generateDependency(secondPackage, "1.0.0", 0, null), + generateDependency(thirdPackage, "1.0.0", 0, null) + ]; + + const pathToSecondPackageInsideFirstPackage = path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, secondPackage); + const expectedResult: IDependencyData[] = [ + getNodeModuleInfoForExpecteDependency(firstPackage, 0), + getNodeModuleInfoForExpecteDependency(pathToSecondPackageInsideFirstPackage, 1), + getNodeModuleInfoForExpecteDependency(secondPackage, 0), + getNodeModuleInfoForExpecteDependency(thirdPackage, 0), + getNodeModuleInfoForExpecteDependency(path.join(pathToSecondPackageInsideFirstPackage, constants.NODE_MODULES_FOLDER_NAME, thirdPackage), 2), + getNodeModuleInfoForExpecteDependency(path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, thirdPackage), 1) + ]; + + const nodeModulesDependenciesBuilder = generateTest(rootDeps); + const actualResult = await nodeModulesDependenciesBuilder.getProductionDependencies(pathToProject); + assert.deepEqual(actualResult, expectedResult); + }); + + it("when the installed packages have nativescript data in their package.json", async () => { + // The test validates the following dependency tree, when npm 3+ is used. + // + // ├── firstPackage@1.0.0 + // ├── secondPackage@1.1.0 + // └── thirdPackage@1.2.0 + + const getNativeScriptDataForPlugin = (pluginName: string): any => { + return { + platforms: { + "tns-android": "x.x.x", + "tns-ios": "x.x.x", + }, + + customPropertyUsedForThisTestOnly: pluginName + }; + }; + + const rootDeps: IDependencyInfo[] = [ + generateDependency(firstPackage, "1.0.0", 0, null, getNativeScriptDataForPlugin(firstPackage)), + generateDependency(secondPackage, "1.1.0", 0, null, getNativeScriptDataForPlugin(secondPackage)), + generateDependency(thirdPackage, "1.2.0", 0, null, getNativeScriptDataForPlugin(thirdPackage)) + ]; + + const nodeModulesDependenciesBuilder = generateTest(rootDeps); + const actualResult = await nodeModulesDependenciesBuilder.getProductionDependencies(pathToProject); + + const expectedResult: IDependencyData[] = [ + getNodeModuleInfoForExpecteDependency(firstPackage, 0, getNativeScriptDataForPlugin(firstPackage)), + getNodeModuleInfoForExpecteDependency(secondPackage, 0, getNativeScriptDataForPlugin(secondPackage)), + getNodeModuleInfoForExpecteDependency(thirdPackage, 0, getNativeScriptDataForPlugin(thirdPackage)) + ]; + + assert.deepEqual(actualResult, expectedResult); + }); + }); + }); +}); From 9609860614d738e1dfe4777a07dcd85534663a6b Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Thu, 11 May 2017 11:32:26 +0300 Subject: [PATCH 2/3] 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. --- lib/declarations.d.ts | 7 +- .../node-modules-dependencies-builder.ts | 83 ++++++++++--------- .../node-modules-dependencies-builder.ts | 35 ++++---- 3 files changed, 66 insertions(+), 59 deletions(-) diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index 8760c4d2ec..bff60a8780 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -203,7 +203,7 @@ interface INpmInstallOptions { } /** - * Describes a package installed in node_modules directory of a project. + * Describes npm package installed in node_modules. */ interface IDependencyData { /** @@ -227,6 +227,11 @@ interface IDependencyData { * Describes the `nativescript` key in package.json of a dependency. */ nativescript?: any; + + /** + * Dependencies of the current module. + */ + dependencies?: string[]; } interface IStaticConfig extends Config.IStaticConfig { } diff --git a/lib/tools/node-modules/node-modules-dependencies-builder.ts b/lib/tools/node-modules/node-modules-dependencies-builder.ts index 6011743e2b..f4eea9a3c0 100644 --- a/lib/tools/node-modules/node-modules-dependencies-builder.ts +++ b/lib/tools/node-modules/node-modules-dependencies-builder.ts @@ -1,6 +1,12 @@ import * as path from "path"; import { NODE_MODULES_FOLDER_NAME, PACKAGE_JSON_FILE_NAME } from "../../constants"; +interface IDependencyDescription { + parentDir: string; + name: string; + depth: number; +} + export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesBuilder { public constructor(private $fs: IFileSystem) { } @@ -12,51 +18,56 @@ export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesB let resolvedDependencies: IDependencyData[] = []; - _.keys(dependencies) - .forEach(dependencyName => { - const depth = 0; - const directory = path.join(rootNodeModulesPath, dependencyName); - - // find and traverse child with name `key`, parent's directory -> dep.directory - this.traverseDependency(resolvedDependencies, rootNodeModulesPath, dependencyName, directory, depth); + let queue: IDependencyDescription[] = _.keys(dependencies) + .map(dependencyName => { + return { + parentDir: projectPath, + name: dependencyName, + depth: 0 + }; }); - return resolvedDependencies; - } + while (queue.length) { + const currentModule = queue.shift(); + const resolvedDependency = this.findModule(rootNodeModulesPath, currentModule.parentDir, currentModule.name, currentModule.depth); - private traverseDependency(resolvedDependencies: IDependencyData[], rootNodeModulesPath: string, 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 - const modulePath = path.join(currentModulePath, NODE_MODULES_FOLDER_NAME, name); // node_modules/parent/node_modules/ - const alternativeModulePath = path.join(rootNodeModulesPath, name); + if (resolvedDependency && !_.some(resolvedDependencies, r => r.directory === resolvedDependency.directory)) { + const deps = _.map(resolvedDependency.dependencies, d => ({ name: d, parentDir: resolvedDependency.directory, depth: resolvedDependency.depth + 1 })); + queue.push(...deps); + resolvedDependencies.push(resolvedDependency); + } + } - this.findModule(resolvedDependencies, rootNodeModulesPath, modulePath, alternativeModulePath, name, depth); + return resolvedDependencies; } - private findModule(resolvedDependencies: IDependencyData[], rootNodeModulesPath: string, modulePath: string, rootModulesPath: string, name: string, depth: number): void { + private findModule(rootNodeModulesPath: string, parentModulePath: string, name: string, depth: number): IDependencyData { + let modulePath = path.join(parentModulePath, NODE_MODULES_FOLDER_NAME, name); // node_modules/parent/node_modules/ + const rootModulesPath = path.join(rootNodeModulesPath, name); let exists = this.moduleExists(modulePath); - let depthInNodeModules = depth + 1; + let depthInNodeModules = depth; if (!exists) { modulePath = rootModulesPath; // /node_modules/ exists = this.moduleExists(modulePath); - depthInNodeModules = 0; } - if (!exists || _.some(resolvedDependencies, deps => deps.directory === modulePath)) { - return; + if (!exists) { + return null; } - const dependency = this.getDependencyInfo(name, modulePath, depthInNodeModules); - resolvedDependencies.push(dependency); - - this.readModuleDependencies(resolvedDependencies, rootNodeModulesPath, modulePath, depthInNodeModules, dependency); + return this.getDependencyData(name, modulePath, depthInNodeModules); } - private readModuleDependencies(resolvedDependencies: IDependencyData[], rootNodeModulesPath: string, modulePath: string, depth: number, currentModule: IDependencyData): void { - const packageJsonPath = path.join(modulePath, PACKAGE_JSON_FILE_NAME); + 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) { @@ -64,24 +75,14 @@ export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesB 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(resolvedDependencies, rootNodeModulesPath, dependencyName, modulePath, depth); - }); + dependency.dependencies = _.keys(packageJsonContents.dependencies); + return dependency; } - } - - private getDependencyInfo(name: string, directory: string, depth: number): IDependencyData { - const dependency: IDependencyData = { - name, - directory, - depth - }; - return dependency; + return null; } private moduleExists(modulePath: string): boolean { diff --git a/test/tools/node-modules/node-modules-dependencies-builder.ts b/test/tools/node-modules/node-modules-dependencies-builder.ts index 7850839528..fb25057a12 100644 --- a/test/tools/node-modules/node-modules-dependencies-builder.ts +++ b/test/tools/node-modules/node-modules-dependencies-builder.ts @@ -54,15 +54,16 @@ describe("nodeModulesDependenciesBuilder", () => { * Helper functions for easier writing of consecutive tests in the suite. * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ - const getPathToDependencyInNodeModules = (dependencyName: string): string => { - return path.join(pathToProject, constants.NODE_MODULES_FOLDER_NAME, dependencyName); + const getPathToDependencyInNodeModules = (dependencyName: string, parentDir?: string): string => { + return path.join(parentDir || pathToProject, constants.NODE_MODULES_FOLDER_NAME, dependencyName); }; - const getNodeModuleInfoForExpecteDependency = (name: string, depth: number, nativescript?: any): IDependencyData => { - let result: IDependencyData = { + const getNodeModuleInfoForExpecteDependency = (name: string, depth: number, nativescript?: any, dependencies?: string[]): IDependencyData => { + let result: IDependencyData = { name: path.basename(name), directory: getPathToDependencyInNodeModules(name), - depth + depth, + dependencies: dependencies || [] }; if (nativescript) { @@ -72,8 +73,8 @@ describe("nodeModulesDependenciesBuilder", () => { return result; }; - const getPathToPackageJsonOfDependency = (dependencyName: string): string => { - return path.join(getPathToDependencyInNodeModules(dependencyName), constants.PACKAGE_JSON_FILE_NAME); + const getPathToPackageJsonOfDependency = (dependencyName: string, parentDir?: string): string => { + return path.join(getPathToDependencyInNodeModules(dependencyName, parentDir), constants.PACKAGE_JSON_FILE_NAME); }; const getDependenciesObjectFromDependencyInfo = (depInfos: IDependencyInfo[], nativescript: any): { dependencies: any, nativescript?: any } => { @@ -93,16 +94,16 @@ describe("nodeModulesDependenciesBuilder", () => { return result; }; - const getDependenciesObject = (filename: string, deps: IDependencyInfo[]): { dependencies: any } => { + const getDependenciesObject = (filename: string, deps: IDependencyInfo[], parentDir: string): { dependencies: any } => { let result: { dependencies: any } = null; for (let dependencyInfo of deps) { - const pathToPackageJson = getPathToPackageJsonOfDependency(dependencyInfo.name); + const pathToPackageJson = getPathToPackageJsonOfDependency(dependencyInfo.name, parentDir); if (filename === pathToPackageJson) { return getDependenciesObjectFromDependencyInfo(dependencyInfo.dependencies, dependencyInfo.nativescript); } if (dependencyInfo.dependencies) { - result = getDependenciesObject(filename, dependencyInfo.dependencies); + result = getDependenciesObject(filename, dependencyInfo.dependencies, path.join(parentDir, constants.NODE_MODULES_FOLDER_NAME, dependencyInfo.name)); if (result) { break; } @@ -118,7 +119,7 @@ describe("nodeModulesDependenciesBuilder", () => { const fs = testInjector.resolve("fs"); fs.readJson = (filename: string, encoding?: string): any => { - const innerDependency = getDependenciesObject(filename, rootDeps); + const innerDependency = getDependenciesObject(filename, rootDeps, pathToProject); return innerDependency || getDependenciesObjectFromDependencyInfo(rootDeps, null); }; @@ -232,9 +233,9 @@ describe("nodeModulesDependenciesBuilder", () => { ]; const expectedResult: IDependencyData[] = [ - getNodeModuleInfoForExpecteDependency(firstPackage, 0), - getNodeModuleInfoForExpecteDependency(path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, secondPackage), 1), - getNodeModuleInfoForExpecteDependency(secondPackage, 0) + getNodeModuleInfoForExpecteDependency(firstPackage, 0, null, [secondPackage]), + getNodeModuleInfoForExpecteDependency(secondPackage, 0), + getNodeModuleInfoForExpecteDependency(path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, secondPackage), 1) ]; const nodeModulesDependenciesBuilder = generateTest(rootDeps); @@ -265,12 +266,12 @@ describe("nodeModulesDependenciesBuilder", () => { const pathToSecondPackageInsideFirstPackage = path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, secondPackage); const expectedResult: IDependencyData[] = [ - getNodeModuleInfoForExpecteDependency(firstPackage, 0), - getNodeModuleInfoForExpecteDependency(pathToSecondPackageInsideFirstPackage, 1), + getNodeModuleInfoForExpecteDependency(firstPackage, 0, null, [secondPackage, thirdPackage]), getNodeModuleInfoForExpecteDependency(secondPackage, 0), getNodeModuleInfoForExpecteDependency(thirdPackage, 0), + getNodeModuleInfoForExpecteDependency(pathToSecondPackageInsideFirstPackage, 1, null, [thirdPackage]), + getNodeModuleInfoForExpecteDependency(path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, thirdPackage), 1), getNodeModuleInfoForExpecteDependency(path.join(pathToSecondPackageInsideFirstPackage, constants.NODE_MODULES_FOLDER_NAME, thirdPackage), 2), - getNodeModuleInfoForExpecteDependency(path.join(firstPackage, constants.NODE_MODULES_FOLDER_NAME, thirdPackage), 1) ]; const nodeModulesDependenciesBuilder = generateTest(rootDeps); From 4b65f597958fd5979cb52562d99a1f0948b720f8 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Thu, 11 May 2017 17:16:20 +0300 Subject: [PATCH 3/3] 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". --- .../node-modules-dependencies-builder.ts | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/lib/tools/node-modules/node-modules-dependencies-builder.ts b/lib/tools/node-modules/node-modules-dependencies-builder.ts index f4eea9a3c0..396eb7dc34 100644 --- a/lib/tools/node-modules/node-modules-dependencies-builder.ts +++ b/lib/tools/node-modules/node-modules-dependencies-builder.ts @@ -19,21 +19,30 @@ export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesB let resolvedDependencies: IDependencyData[] = []; let queue: IDependencyDescription[] = _.keys(dependencies) - .map(dependencyName => { - return { - parentDir: projectPath, - name: dependencyName, - depth: 0 - }; - }); + .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); + const resolvedDependency = this.findModule(rootNodeModulesPath, currentModule.parentDir, currentModule.name, currentModule.depth, resolvedDependencies); if (resolvedDependency && !_.some(resolvedDependencies, r => r.directory === resolvedDependency.directory)) { - const deps = _.map(resolvedDependency.dependencies, d => ({ name: d, parentDir: resolvedDependency.directory, depth: resolvedDependency.depth + 1 })); - queue.push(...deps); + _.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); } } @@ -41,20 +50,23 @@ export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesB return resolvedDependencies; } - private findModule(rootNodeModulesPath: string, parentModulePath: string, name: string, depth: number): IDependencyData { + 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/ const rootModulesPath = path.join(rootNodeModulesPath, name); - let exists = this.moduleExists(modulePath); let depthInNodeModules = depth; - if (!exists) { + if (!this.moduleExists(modulePath)) { modulePath = rootModulesPath; // /node_modules/ - exists = this.moduleExists(modulePath); + if (!this.moduleExists(modulePath)) { + return null; + } + depthInNodeModules = 0; } - if (!exists) { + if (_.some(resolvedDependencies, r => r.name === name && r.directory === modulePath)) { return null; + } return this.getDependencyData(name, modulePath, depthInNodeModules);